netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH ethtool-next] netlink: fec: add errors histogram statistics
@ 2025-10-03 22:52 Vadim Fedorenko
  2025-10-04  9:53 ` [External] : " ALOK TIWARI
  0 siblings, 1 reply; 3+ messages in thread
From: Vadim Fedorenko @ 2025-10-03 22:52 UTC (permalink / raw)
  To: mkubecek; +Cc: Vadim Fedorenko, Jakub Kicinski, netdev

Linux 6.18 has FEC errors histogram statistics API added. Add support
for extra attributes in ethtool.

 # ethtool -I --show-fec eni8np1
FEC parameters for eni8np1:
Supported/Configured FEC encodings: None
Active FEC encoding: None
Statistics:
  corrected_blocks: 123
  uncorrectable_blocks: 4
  fec_bit_err_0: 445 [ per_lane:  125, 120, 100, 100 ]
  fec_bit_err_1_to_3: 12
  fec_bit_err_4_to_7: 2

 # ethtool -j -I --show-fec eni8np1
[ {
        "ifname": "eni8np1",
        "config": [ "None" ],
        "active": [ "None" ],
        "statistics": {
            "corrected_blocks": {
                "total": 123
            },
            "uncorrectable_blocks": {
                "total": 4
            },
            "hist": [ {
                    "bin_low": 0,
                    "bin_high": 0,
                    "total": 445,
                    "lanes": [ 125,120,100,100 ]
                },{
                    "bin_low": 1,
                    "bin_high": 3,
                    "total": 12
                },{
                    "bin_low": 4,
                    "bin_high": 7,
                    "total": 2
                } ]
        }
    } ]

Signed-off-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
---
 netlink/fec.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 72 insertions(+)

diff --git a/netlink/fec.c b/netlink/fec.c
index ed100d7..32f7ca7 100644
--- a/netlink/fec.c
+++ b/netlink/fec.c
@@ -44,6 +44,64 @@ fec_mode_walk(unsigned int idx, const char *name, bool val, void *data)
 	print_string(PRINT_ANY, NULL, " %s", name);
 }
 
+static void fec_show_hist_bin(const struct nlattr *hist)
+{
+	const struct nlattr *tb[ETHTOOL_A_FEC_HIST_MAX + 1] = {};
+	DECLARE_ATTR_TB_INFO(tb);
+	unsigned int i, lanes, bin_high, bin_low;
+	uint64_t val, *vals;
+	int ret;
+
+	ret = mnl_attr_parse_nested(hist, attr_cb, &tb_info);
+	if (ret < 0)
+		return;
+
+	if (!tb[ETHTOOL_A_FEC_HIST_BIN_LOW] || !tb[ETHTOOL_A_FEC_HIST_BIN_HIGH])
+		return;
+
+	bin_high = mnl_attr_get_u32(tb[ETHTOOL_A_FEC_HIST_BIN_HIGH]);
+	bin_low  = mnl_attr_get_u32(tb[ETHTOOL_A_FEC_HIST_BIN_LOW]);
+	/* Bin value is uint, so it may be u32 or u64 depeding on the value */
+	if (mnl_attr_validate(tb[ETHTOOL_A_FEC_HIST_BIN_VAL], MNL_TYPE_U32) < 0)
+		val = mnl_attr_get_u64(tb[ETHTOOL_A_FEC_HIST_BIN_VAL]);
+	else
+		val = mnl_attr_get_u32(tb[ETHTOOL_A_FEC_HIST_BIN_VAL]);
+
+	if (is_json_context()) {
+		print_u64(PRINT_JSON, "bin_low", NULL, bin_low);
+		print_u64(PRINT_JSON, "bin_high", NULL, bin_high);
+		print_u64(PRINT_JSON, "total", NULL, val);
+	} else {
+		printf("  fec_bit_err_%d", bin_low);
+		if (bin_low != bin_high)
+			printf("_to_%d", bin_high);
+		printf(": %" PRIu64, val);
+	}
+	if (!tb[ETHTOOL_A_FEC_HIST_BIN_VAL_PER_LANE]) {
+		if (!is_json_context())
+			print_nl();
+		return;
+	}
+
+	vals = mnl_attr_get_payload(tb[ETHTOOL_A_FEC_HIST_BIN_VAL_PER_LANE]);
+	lanes = mnl_attr_get_payload_len(tb[ETHTOOL_A_FEC_HIST_BIN_VAL_PER_LANE]) / 8;
+	if (is_json_context())
+		open_json_array("lanes", "");
+	else
+		printf(" [ per_lane:");
+	for (i = 0; i < lanes; i++) {
+		if (is_json_context())
+			print_u64(PRINT_JSON, NULL, NULL, *vals++);
+		else
+			printf("%s %" PRIu64, i ? "," : "", *vals++);
+	}
+
+	if (is_json_context())
+		close_json_array("");
+	else
+		printf(" ]\n");
+}
+
 static int fec_show_stats(const struct nlattr *nest)
 {
 	const struct nlattr *tb[ETHTOOL_A_FEC_STAT_MAX + 1] = {};
@@ -108,6 +166,20 @@ static int fec_show_stats(const struct nlattr *nest)
 
 		close_json_object();
 	}
+
+	if (tb[ETHTOOL_A_FEC_STAT_HIST]) {
+		const struct nlattr *attr;
+
+		open_json_array("hist", "");
+		mnl_attr_for_each_nested(attr, nest) {
+			if (mnl_attr_get_type(attr) == ETHTOOL_A_FEC_STAT_HIST) {
+				open_json_object(NULL);
+				fec_show_hist_bin(attr);
+				close_json_object();
+			}
+		}
+		close_json_array("");
+	}
 	close_json_object();
 
 	return 0;
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [External] : [PATCH ethtool-next] netlink: fec: add errors histogram statistics
  2025-10-03 22:52 [PATCH ethtool-next] netlink: fec: add errors histogram statistics Vadim Fedorenko
@ 2025-10-04  9:53 ` ALOK TIWARI
  2025-10-04 12:33   ` Vadim Fedorenko
  0 siblings, 1 reply; 3+ messages in thread
From: ALOK TIWARI @ 2025-10-04  9:53 UTC (permalink / raw)
  To: Vadim Fedorenko, mkubecek; +Cc: Jakub Kicinski, netdev



On 10/4/2025 4:22 AM, Vadim Fedorenko wrote:
> Linux 6.18 has FEC errors histogram statistics API added. Add support
> for extra attributes in ethtool.
> 
>   # ethtool -I --show-fec eni8np1
> FEC parameters for eni8np1:
> Supported/Configured FEC encodings: None
> Active FEC encoding: None
> Statistics:
>    corrected_blocks: 123
>    uncorrectable_blocks: 4
>    fec_bit_err_0: 445 [ per_lane:  125, 120, 100, 100 ]
>    fec_bit_err_1_to_3: 12
>    fec_bit_err_4_to_7: 2
> 
>   # ethtool -j -I --show-fec eni8np1
> [ {
>          "ifname": "eni8np1",
>          "config": [ "None" ],
>          "active": [ "None" ],
>          "statistics": {
>              "corrected_blocks": {
>                  "total": 123
>              },
>              "uncorrectable_blocks": {
>                  "total": 4
>              },
>              "hist": [ {
>                      "bin_low": 0,
>                      "bin_high": 0,
>                      "total": 445,
>                      "lanes": [ 125,120,100,100 ]
>                  },{
>                      "bin_low": 1,
>                      "bin_high": 3,
>                      "total": 12
>                  },{
>                      "bin_low": 4,
>                      "bin_high": 7,
>                      "total": 2
>                  } ]
>          }
>      } ]
> 
> Signed-off-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
> ---
>   netlink/fec.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 72 insertions(+)
> 
> diff --git a/netlink/fec.c b/netlink/fec.c
> index ed100d7..32f7ca7 100644
> --- a/netlink/fec.c
> +++ b/netlink/fec.c
> @@ -44,6 +44,64 @@ fec_mode_walk(unsigned int idx, const char *name, bool val, void *data)
>   	print_string(PRINT_ANY, NULL, " %s", name);
>   }
>   
> +static void fec_show_hist_bin(const struct nlattr *hist)
> +{
> +	const struct nlattr *tb[ETHTOOL_A_FEC_HIST_MAX + 1] = {};
> +	DECLARE_ATTR_TB_INFO(tb);
> +	unsigned int i, lanes, bin_high, bin_low;
> +	uint64_t val, *vals;
> +	int ret;
> +
> +	ret = mnl_attr_parse_nested(hist, attr_cb, &tb_info);
> +	if (ret < 0)
> +		return;
> +
> +	if (!tb[ETHTOOL_A_FEC_HIST_BIN_LOW] || !tb[ETHTOOL_A_FEC_HIST_BIN_HIGH])
> +		return;
> +
> +	bin_high = mnl_attr_get_u32(tb[ETHTOOL_A_FEC_HIST_BIN_HIGH]);
> +	bin_low  = mnl_attr_get_u32(tb[ETHTOOL_A_FEC_HIST_BIN_LOW]);
> +	/* Bin value is uint, so it may be u32 or u64 depeding on the value */

typo depeding -> depending

> +	if (mnl_attr_validate(tb[ETHTOOL_A_FEC_HIST_BIN_VAL], MNL_TYPE_U32) < 0)
> +		val = mnl_attr_get_u64(tb[ETHTOOL_A_FEC_HIST_BIN_VAL]);
> +	else
> +		val = mnl_attr_get_u32(tb[ETHTOOL_A_FEC_HIST_BIN_VAL]);
> +
> +	if (is_json_context()) {
> +		print_u64(PRINT_JSON, "bin_low", NULL, bin_low);
> +		print_u64(PRINT_JSON, "bin_high", NULL, bin_high);
> +		print_u64(PRINT_JSON, "total", NULL, val);
> +	} else {
> +		printf("  fec_bit_err_%d", bin_low);
> +		if (bin_low != bin_high)
> +			printf("_to_%d", bin_high);
> +		printf(": %" PRIu64, val);
> +	}
> +	if (!tb[ETHTOOL_A_FEC_HIST_BIN_VAL_PER_LANE]) {
> +		if (!is_json_context())
> +			print_nl();
> +		return;
> +	}
> +
> +	vals = mnl_attr_get_payload(tb[ETHTOOL_A_FEC_HIST_BIN_VAL_PER_LANE]);
> +	lanes = mnl_attr_get_payload_len(tb[ETHTOOL_A_FEC_HIST_BIN_VAL_PER_LANE]) / 8;

8 -> sizeof(uint64_t)

> +	if (is_json_context())
> +		open_json_array("lanes", "");
> +	else
> +		printf(" [ per_lane:");
> +	for (i = 0; i < lanes; i++) {
> +		if (is_json_context())
> +			print_u64(PRINT_JSON, NULL, NULL, *vals++);
> +		else
> +			printf("%s %" PRIu64, i ? "," : "", *vals++);

"" -> " "

> +	}
> +
> +	if (is_json_context())
> +		close_json_array("");
> +	else
> +		printf(" ]\n");
> +}
> +
>   static int fec_show_stats(const struct nlattr *nest)
>   {
>   	const struct nlattr *tb[ETHTOOL_A_FEC_STAT_MAX + 1] = {};
> @@ -108,6 +166,20 @@ static int fec_show_stats(const struct nlattr *nest)
>   
>   		close_json_object();
>   	}
> +
> +	if (tb[ETHTOOL_A_FEC_STAT_HIST]) {
> +		const struct nlattr *attr;
> +
> +		open_json_array("hist", "");
> +		mnl_attr_for_each_nested(attr, nest) {

"mnl_attr_for_each_nested(attr, nest) {" or 
"mnl_attr_for_each_nested(attr, tb[ETHTOOL_A_FEC_STAT_HIST]) {" ? please 
check it

> +			if (mnl_attr_get_type(attr) == ETHTOOL_A_FEC_STAT_HIST) {
> +				open_json_object(NULL);
> +				fec_show_hist_bin(attr);
> +				close_json_object();
> +			}
> +		}
> +		close_json_array("");
> +	}
>   	close_json_object();
>   
>   	return 0;

Thanks,
Alok


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [External] : [PATCH ethtool-next] netlink: fec: add errors histogram statistics
  2025-10-04  9:53 ` [External] : " ALOK TIWARI
@ 2025-10-04 12:33   ` Vadim Fedorenko
  0 siblings, 0 replies; 3+ messages in thread
From: Vadim Fedorenko @ 2025-10-04 12:33 UTC (permalink / raw)
  To: ALOK TIWARI, mkubecek; +Cc: Jakub Kicinski, netdev

On 04/10/2025 10:53, ALOK TIWARI wrote:
> 
> 
> On 10/4/2025 4:22 AM, Vadim Fedorenko wrote:
>> Linux 6.18 has FEC errors histogram statistics API added. Add support
>> for extra attributes in ethtool.
>>
>>   # ethtool -I --show-fec eni8np1
>> FEC parameters for eni8np1:
>> Supported/Configured FEC encodings: None
>> Active FEC encoding: None
>> Statistics:
>>    corrected_blocks: 123
>>    uncorrectable_blocks: 4
>>    fec_bit_err_0: 445 [ per_lane:  125, 120, 100, 100 ]
>>    fec_bit_err_1_to_3: 12
>>    fec_bit_err_4_to_7: 2
>>
>>   # ethtool -j -I --show-fec eni8np1
>> [ {
>>          "ifname": "eni8np1",
>>          "config": [ "None" ],
>>          "active": [ "None" ],
>>          "statistics": {
>>              "corrected_blocks": {
>>                  "total": 123
>>              },
>>              "uncorrectable_blocks": {
>>                  "total": 4
>>              },
>>              "hist": [ {
>>                      "bin_low": 0,
>>                      "bin_high": 0,
>>                      "total": 445,
>>                      "lanes": [ 125,120,100,100 ]
>>                  },{
>>                      "bin_low": 1,
>>                      "bin_high": 3,
>>                      "total": 12
>>                  },{
>>                      "bin_low": 4,
>>                      "bin_high": 7,
>>                      "total": 2
>>                  } ]
>>          }
>>      } ]
>>
>> Signed-off-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
>> ---
>>   netlink/fec.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 72 insertions(+)
>>
>> diff --git a/netlink/fec.c b/netlink/fec.c
>> index ed100d7..32f7ca7 100644
>> --- a/netlink/fec.c
>> +++ b/netlink/fec.c
>> @@ -44,6 +44,64 @@ fec_mode_walk(unsigned int idx, const char *name, 
>> bool val, void *data)
>>       print_string(PRINT_ANY, NULL, " %s", name);
>>   }
>> +static void fec_show_hist_bin(const struct nlattr *hist)
>> +{
>> +    const struct nlattr *tb[ETHTOOL_A_FEC_HIST_MAX + 1] = {};
>> +    DECLARE_ATTR_TB_INFO(tb);
>> +    unsigned int i, lanes, bin_high, bin_low;
>> +    uint64_t val, *vals;
>> +    int ret;
>> +
>> +    ret = mnl_attr_parse_nested(hist, attr_cb, &tb_info);
>> +    if (ret < 0)
>> +        return;
>> +
>> +    if (!tb[ETHTOOL_A_FEC_HIST_BIN_LOW] || ! 
>> tb[ETHTOOL_A_FEC_HIST_BIN_HIGH])
>> +        return;
>> +
>> +    bin_high = mnl_attr_get_u32(tb[ETHTOOL_A_FEC_HIST_BIN_HIGH]);
>> +    bin_low  = mnl_attr_get_u32(tb[ETHTOOL_A_FEC_HIST_BIN_LOW]);
>> +    /* Bin value is uint, so it may be u32 or u64 depeding on the 
>> value */
> 
> typo depeding -> depending

yep, missed it..

> 
>> +    if (mnl_attr_validate(tb[ETHTOOL_A_FEC_HIST_BIN_VAL], 
>> MNL_TYPE_U32) < 0)
>> +        val = mnl_attr_get_u64(tb[ETHTOOL_A_FEC_HIST_BIN_VAL]);
>> +    else
>> +        val = mnl_attr_get_u32(tb[ETHTOOL_A_FEC_HIST_BIN_VAL]);
>> +
>> +    if (is_json_context()) {
>> +        print_u64(PRINT_JSON, "bin_low", NULL, bin_low);
>> +        print_u64(PRINT_JSON, "bin_high", NULL, bin_high);
>> +        print_u64(PRINT_JSON, "total", NULL, val);
>> +    } else {
>> +        printf("  fec_bit_err_%d", bin_low);
>> +        if (bin_low != bin_high)
>> +            printf("_to_%d", bin_high);
>> +        printf(": %" PRIu64, val);
>> +    }
>> +    if (!tb[ETHTOOL_A_FEC_HIST_BIN_VAL_PER_LANE]) {
>> +        if (!is_json_context())
>> +            print_nl();
>> +        return;
>> +    }
>> +
>> +    vals = 
>> mnl_attr_get_payload(tb[ETHTOOL_A_FEC_HIST_BIN_VAL_PER_LANE]);
>> +    lanes = 
>> mnl_attr_get_payload_len(tb[ETHTOOL_A_FEC_HIST_BIN_VAL_PER_LANE]) / 8;
> 
> 8 -> sizeof(uint64_t)
There is no history of using sizeof(uint64_t) in code base, but the 
codearound uses "/ 8" and "% 8" for u64 attributes.

>> +    if (is_json_context())
>> +        open_json_array("lanes", "");
>> +    else
>> +        printf(" [ per_lane:");
>> +    for (i = 0; i < lanes; i++) {
>> +        if (is_json_context())
>> +            print_u64(PRINT_JSON, NULL, NULL, *vals++);
>> +        else
>> +            printf("%s %" PRIu64, i ? "," : "", *vals++);
> 
> "" -> " "
That will break formatting. Empty string is intended here.

> 
>> +    }
>> +
>> +    if (is_json_context())
>> +        close_json_array("");
>> +    else
>> +        printf(" ]\n");
>> +}
>> +
>>   static int fec_show_stats(const struct nlattr *nest)
>>   {
>>       const struct nlattr *tb[ETHTOOL_A_FEC_STAT_MAX + 1] = {};
>> @@ -108,6 +166,20 @@ static int fec_show_stats(const struct nlattr *nest)
>>           close_json_object();
>>       }
>> +
>> +    if (tb[ETHTOOL_A_FEC_STAT_HIST]) {
>> +        const struct nlattr *attr;
>> +
>> +        open_json_array("hist", "");
>> +        mnl_attr_for_each_nested(attr, nest) {
> 
> "mnl_attr_for_each_nested(attr, nest) {" or 
> "mnl_attr_for_each_nested(attr, tb[ETHTOOL_A_FEC_STAT_HIST]) {" ? please 
> check it

tb[ETHTOOL_A_FEC_STAT_HIST] will not have nested attributes of type
ETHTOOL_A_FEC_STAT_HIST, the correct way is the one in the patch

>> +            if (mnl_attr_get_type(attr) == ETHTOOL_A_FEC_STAT_HIST) {
>> +                open_json_object(NULL);
>> +                fec_show_hist_bin(attr);
>> +                close_json_object();
>> +            }
>> +        }
>> +        close_json_array("");
>> +    }
>>       close_json_object();
>>       return 0;
> 
> Thanks,
> Alok
> 


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2025-10-04 12:33 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-03 22:52 [PATCH ethtool-next] netlink: fec: add errors histogram statistics Vadim Fedorenko
2025-10-04  9:53 ` [External] : " ALOK TIWARI
2025-10-04 12:33   ` Vadim Fedorenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).