| Message ID | 20210415225318.2726095-1-kuba@kernel.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | ethtool: add standard FEC statistics | expand |
On Thu, 2021-04-15 at 15:53 -0700, Jakub Kicinski wrote: > This set adds uAPI for reporting standard FEC statistics, and > implements it in a handful of drivers. > > The statistics are taken from the IEEE standard, with one > extra seemingly popular but not standard statistics added. > > The implementation is similar to that of the pause frame > statistics, user requests the stats by setting a bit > (ETHTOOL_FLAG_STATS) in the common ethtool header of > ETHTOOL_MSG_FEC_GET. > > Since standard defines the statistics per lane what's > reported is both total and per-lane counters: > > # ethtool -I --show-fec eth0 > FEC parameters for eth0: > Configured FEC encodings: None > Active FEC encoding: None > Statistics: > corrected_blocks: 256 > Lane 0: 255 > Lane 1: 1 > uncorrectable_blocks: 145 > Lane 0: 128 > Lane 1: 17 > > v2: check for errors in mlx5 register access > > Jakub Kicinski (6): > ethtool: move ethtool_stats_init > ethtool: fec_prepare_data() - jump to error handling > ethtool: add FEC statistics > bnxt: implement ethtool::get_fec_stats > sfc: ef10: implement ethtool::get_fec_stats > mlx5: implement ethtool::get_fec_stats For the series: Reviewed-by: Saeed Mahameed <saeedm@nvidia.com>
Hello: This series was applied to netdev/net-next.git (refs/heads/master): On Thu, 15 Apr 2021 15:53:12 -0700 you wrote: > This set adds uAPI for reporting standard FEC statistics, and > implements it in a handful of drivers. > > The statistics are taken from the IEEE standard, with one > extra seemingly popular but not standard statistics added. > > The implementation is similar to that of the pause frame > statistics, user requests the stats by setting a bit > (ETHTOOL_FLAG_STATS) in the common ethtool header of > ETHTOOL_MSG_FEC_GET. > > [...] Here is the summary with links: - [net-next,v2,1/6] ethtool: move ethtool_stats_init https://git.kernel.org/netdev/net-next/c/c5797f8a6415 - [net-next,v2,2/6] ethtool: fec_prepare_data() - jump to error handling https://git.kernel.org/netdev/net-next/c/3d7cc109ecf7 - [net-next,v2,3/6] ethtool: add FEC statistics https://git.kernel.org/netdev/net-next/c/be85dbfeb37c - [net-next,v2,4/6] bnxt: implement ethtool::get_fec_stats https://git.kernel.org/netdev/net-next/c/c9ca5c3aabaf - [net-next,v2,5/6] sfc: ef10: implement ethtool::get_fec_stats https://git.kernel.org/netdev/net-next/c/cab351be53c2 - [net-next,v2,6/6] mlx5: implement ethtool::get_fec_stats https://git.kernel.org/netdev/net-next/c/1703bb50df0e You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
On 2021/4/16 6:53, Jakub Kicinski wrote: > This set adds uAPI for reporting standard FEC statistics, and > implements it in a handful of drivers. > > The statistics are taken from the IEEE standard, with one > extra seemingly popular but not standard statistics added. > > The implementation is similar to that of the pause frame > statistics, user requests the stats by setting a bit > (ETHTOOL_FLAG_STATS) in the common ethtool header of > ETHTOOL_MSG_FEC_GET. > > Since standard defines the statistics per lane what's > reported is both total and per-lane counters: > > # ethtool -I --show-fec eth0 > FEC parameters for eth0: > Configured FEC encodings: None > Active FEC encoding: None > Statistics: > corrected_blocks: 256 > Lane 0: 255 > Lane 1: 1 > uncorrectable_blocks: 145 > Lane 0: 128 > Lane 1: 17 > Hi, I have a doubt that why active FEC encoding is None here? Should it actually be BaseR or RS if FEC statistics are reported? > v2: check for errors in mlx5 register access > > Jakub Kicinski (6): > ethtool: move ethtool_stats_init > ethtool: fec_prepare_data() - jump to error handling > ethtool: add FEC statistics > bnxt: implement ethtool::get_fec_stats > sfc: ef10: implement ethtool::get_fec_stats > mlx5: implement ethtool::get_fec_stats > > Documentation/networking/ethtool-netlink.rst | 21 +++++ > Documentation/networking/statistics.rst | 2 + > .../net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 15 ++++ > .../ethernet/mellanox/mlx5/core/en_ethtool.c | 9 +++ > .../ethernet/mellanox/mlx5/core/en_stats.c | 29 ++++++- > .../ethernet/mellanox/mlx5/core/en_stats.h | 2 + > drivers/net/ethernet/sfc/ef10.c | 17 ++++ > drivers/net/ethernet/sfc/ethtool.c | 10 +++ > drivers/net/ethernet/sfc/net_driver.h | 3 + > include/linux/ethtool.h | 46 +++++++++++ > include/uapi/linux/ethtool_netlink.h | 14 ++++ > net/ethtool/fec.c | 80 ++++++++++++++++++- > net/ethtool/pause.c | 6 -- > 13 files changed, 242 insertions(+), 12 deletions(-) >
On Tue, 18 May 2021 14:48:13 +0800 huangguangbin (A) wrote: > On 2021/4/16 6:53, Jakub Kicinski wrote: > > This set adds uAPI for reporting standard FEC statistics, and > > implements it in a handful of drivers. > > > > The statistics are taken from the IEEE standard, with one > > extra seemingly popular but not standard statistics added. > > > > The implementation is similar to that of the pause frame > > statistics, user requests the stats by setting a bit > > (ETHTOOL_FLAG_STATS) in the common ethtool header of > > ETHTOOL_MSG_FEC_GET. > > > > Since standard defines the statistics per lane what's > > reported is both total and per-lane counters: > > > > # ethtool -I --show-fec eth0 > > FEC parameters for eth0: > > Configured FEC encodings: None > > Active FEC encoding: None > > Statistics: > > corrected_blocks: 256 > > Lane 0: 255 > > Lane 1: 1 > > uncorrectable_blocks: 145 > > Lane 0: 128 > > Lane 1: 17 > > Hi, I have a doubt that why active FEC encoding is None here? > Should it actually be BaseR or RS if FEC statistics are reported? Hi! Good point. The values in the example are collected from a netdevsim based mock up which I used for testing the interface, not real hardware. In reality seeing None and corrected/uncorrectable blocks is not valid. That said please keep in mind that the statistics should not be reset when settings are changed, so OFF + stats may happen.
On 2021/5/19 1:30, Jakub Kicinski wrote: > On Tue, 18 May 2021 14:48:13 +0800 huangguangbin (A) wrote: >> On 2021/4/16 6:53, Jakub Kicinski wrote: >>> This set adds uAPI for reporting standard FEC statistics, and >>> implements it in a handful of drivers. >>> >>> The statistics are taken from the IEEE standard, with one >>> extra seemingly popular but not standard statistics added. >>> >>> The implementation is similar to that of the pause frame >>> statistics, user requests the stats by setting a bit >>> (ETHTOOL_FLAG_STATS) in the common ethtool header of >>> ETHTOOL_MSG_FEC_GET. >>> >>> Since standard defines the statistics per lane what's >>> reported is both total and per-lane counters: >>> >>> # ethtool -I --show-fec eth0 >>> FEC parameters for eth0: >>> Configured FEC encodings: None >>> Active FEC encoding: None >>> Statistics: >>> corrected_blocks: 256 >>> Lane 0: 255 >>> Lane 1: 1 >>> uncorrectable_blocks: 145 >>> Lane 0: 128 >>> Lane 1: 17 >> >> Hi, I have a doubt that why active FEC encoding is None here? >> Should it actually be BaseR or RS if FEC statistics are reported? > > Hi! Good point. The values in the example are collected from a netdevsim > based mock up which I used for testing the interface, not real hardware. > In reality seeing None and corrected/uncorrectable blocks is not valid. > That said please keep in mind that the statistics should not be reset > when settings are changed, so OFF + stats may happen. > . > Hi, Jakub, I have another question of per-lane counters. If speed is changed, do the lane number of FEC statistics need to follow the change? For examples, change speed from 200Gbps to 50Gbps, the actual used lane number is changed from 4 to 2, in this case, how many lanes are needed to display FEC statistic? Still 4 lanes or 2 lanes? Thanks, Guangbin .
On Thu, 1 Jul 2021 20:18:15 +0800 huangguangbin (A) wrote: > On 2021/5/19 1:30, Jakub Kicinski wrote: > > On Tue, 18 May 2021 14:48:13 +0800 huangguangbin (A) wrote: > >> Hi, I have a doubt that why active FEC encoding is None here? > >> Should it actually be BaseR or RS if FEC statistics are reported? > > > > Hi! Good point. The values in the example are collected from a netdevsim > > based mock up which I used for testing the interface, not real hardware. > > In reality seeing None and corrected/uncorrectable blocks is not valid. > > That said please keep in mind that the statistics should not be reset > > when settings are changed, so OFF + stats may happen. > > . > > > Hi, Jakub, I have another question of per-lane counters. > If speed is changed, do the lane number of FEC statistics need to follow the change? > For examples, change speed from 200Gbps to 50Gbps, the actual used lane number is > changed from 4 to 2, in this case, how many lanes are needed to display FEC statistic? > Still 4 lanes or 2 lanes? I'd always report counters for max number of lanes supported by the HW. Much like you'd do if you were directly exposing the register values, the registers are still there in HW.
This set adds uAPI for reporting standard FEC statistics, and implements it in a handful of drivers. The statistics are taken from the IEEE standard, with one extra seemingly popular but not standard statistics added. The implementation is similar to that of the pause frame statistics, user requests the stats by setting a bit (ETHTOOL_FLAG_STATS) in the common ethtool header of ETHTOOL_MSG_FEC_GET. Since standard defines the statistics per lane what's reported is both total and per-lane counters: # ethtool -I --show-fec eth0 FEC parameters for eth0: Configured FEC encodings: None Active FEC encoding: None Statistics: corrected_blocks: 256 Lane 0: 255 Lane 1: 1 uncorrectable_blocks: 145 Lane 0: 128 Lane 1: 17 v2: check for errors in mlx5 register access Jakub Kicinski (6): ethtool: move ethtool_stats_init ethtool: fec_prepare_data() - jump to error handling ethtool: add FEC statistics bnxt: implement ethtool::get_fec_stats sfc: ef10: implement ethtool::get_fec_stats mlx5: implement ethtool::get_fec_stats Documentation/networking/ethtool-netlink.rst | 21 +++++ Documentation/networking/statistics.rst | 2 + .../net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 15 ++++ .../ethernet/mellanox/mlx5/core/en_ethtool.c | 9 +++ .../ethernet/mellanox/mlx5/core/en_stats.c | 29 ++++++- .../ethernet/mellanox/mlx5/core/en_stats.h | 2 + drivers/net/ethernet/sfc/ef10.c | 17 ++++ drivers/net/ethernet/sfc/ethtool.c | 10 +++ drivers/net/ethernet/sfc/net_driver.h | 3 + include/linux/ethtool.h | 46 +++++++++++ include/uapi/linux/ethtool_netlink.h | 14 ++++ net/ethtool/fec.c | 80 ++++++++++++++++++- net/ethtool/pause.c | 6 -- 13 files changed, 242 insertions(+), 12 deletions(-)