[PATCH ethtool-next v2 0/7] ethtool: support FEC and standard stats

[PATCH ethtool-next v2 0/7] ethtool: support FEC and standard stats

[Jakub Kicinski]

This series adds support for FEC requests via netlink
and new "standard" stats.

Changes from v1:
 - rebase on next, only conflicts in uAPI update
 - fix the trailing "and" in patch 6
Changes compared to RFC:
 - improve commit messages
 - fix Rx vs Tx histogram in JSON
 - make histograms less hardcoded to RMON
 - expand man page entry for -S a little
 - add --all-groups (last patch)

Jakub Kicinski (7):
  update UAPI header copies
  json: improve array print API
  netlink: add FEC support
  netlink: fec: support displaying statistics
  ethtool: add nlchk for redirecting to netlink
  netlink: add support for standard stats
  netlink: stats: add on --all-groups option

 Makefile.am                  |   3 +-
 ethtool.8.in                 |  23 ++-
 ethtool.c                    |  12 +-
 json_print.c                 |  20 +-
 json_print.h                 |   4 +-
 netlink/desc-ethtool.c       |  51 +++++
 netlink/extapi.h             |  13 +-
 netlink/fec.c                | 359 +++++++++++++++++++++++++++++++++++
 netlink/monitor.c            |   4 +
 netlink/netlink.c            |   9 +-
 netlink/netlink.h            |   1 +
 netlink/parser.c             |  17 +-
 netlink/parser.h             |   4 +
 netlink/stats.c              | 319 +++++++++++++++++++++++++++++++
 uapi/linux/ethtool.h         | 109 +++++++----
 uapi/linux/ethtool_netlink.h | 187 ++++++++++++++++++
 uapi/linux/rtnetlink.h       |  13 ++
 17 files changed, 1094 insertions(+), 54 deletions(-)
 create mode 100644 netlink/fec.c
 create mode 100644 netlink/stats.c

-- 
2.30.2

[PATCH ethtool-next v2 1/7] update UAPI header copies

[Jakub Kicinski]

Update to kernel commit 6ecaf81d4ac6.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 uapi/linux/ethtool.h         | 109 ++++++++++++++------
 uapi/linux/ethtool_netlink.h | 187 +++++++++++++++++++++++++++++++++++
 uapi/linux/rtnetlink.h       |  13 +++
 3 files changed, 277 insertions(+), 32 deletions(-)

diff --git a/uapi/linux/ethtool.h b/uapi/linux/ethtool.h
index a951137bdba9..c6ec1111ffa3 100644
--- a/uapi/linux/ethtool.h
+++ b/uapi/linux/ethtool.h
@@ -24,6 +24,14 @@
  * have the same layout for 32-bit and 64-bit userland.
  */
 
+/* Note on reserved space.
+ * Reserved fields must not be accessed directly by user space because
+ * they may be replaced by a different field in the future. They must
+ * be initialized to zero before making the request, e.g. via memset
+ * of the entire structure or implicitly by not being set in a structure
+ * initializer.
+ */
+
 /**
  * struct ethtool_cmd - DEPRECATED, link control and status
  * This structure is DEPRECATED, please use struct ethtool_link_settings.
@@ -65,6 +73,7 @@
  *	and other link features that the link partner advertised
  *	through autonegotiation; 0 if unknown or not applicable.
  *	Read-only.
+ * @reserved: Reserved for future use; see the note on reserved space.
  *
  * The link speed in Mbps is split between @speed and @speed_hi.  Use
  * the ethtool_cmd_speed() and ethtool_cmd_speed_set() functions to
@@ -153,6 +162,7 @@ static __inline__ __u32 ethtool_cmd_speed(const struct ethtool_cmd *ep)
  * @bus_info: Device bus address.  This should match the dev_name()
  *	string for the underlying bus device, if there is one.  May be
  *	an empty string.
+ * @reserved2: Reserved for future use; see the note on reserved space.
  * @n_priv_flags: Number of flags valid for %ETHTOOL_GPFLAGS and
  *	%ETHTOOL_SPFLAGS commands; also the number of strings in the
  *	%ETH_SS_PRIV_FLAGS set
@@ -354,6 +364,7 @@ struct ethtool_eeprom {
  * @tx_lpi_timer: Time in microseconds the interface delays prior to asserting
  *	its tx lpi (after reaching 'idle' state). Effective only when eee
  *	was negotiated and tx_lpi_enabled was set.
+ * @reserved: Reserved for future use; see the note on reserved space.
  */
 struct ethtool_eee {
 	__u32	cmd;
@@ -372,6 +383,7 @@ struct ethtool_eee {
  * @cmd: %ETHTOOL_GMODULEINFO
  * @type: Standard the module information conforms to %ETH_MODULE_SFF_xxxx
  * @eeprom_len: Length of the eeprom
+ * @reserved: Reserved for future use; see the note on reserved space.
  *
  * This structure is used to return the information to
  * properly size memory for a subsequent call to %ETHTOOL_GMODULEEEPROM.
@@ -577,9 +589,7 @@ struct ethtool_pauseparam {
 	__u32	tx_pause;
 };
 
-/**
- * enum ethtool_link_ext_state - link extended state
- */
+/* Link extended state */
 enum ethtool_link_ext_state {
 	ETHTOOL_LINK_EXT_STATE_AUTONEG,
 	ETHTOOL_LINK_EXT_STATE_LINK_TRAINING_FAILURE,
@@ -593,10 +603,7 @@ enum ethtool_link_ext_state {
 	ETHTOOL_LINK_EXT_STATE_OVERHEAT,
 };
 
-/**
- * enum ethtool_link_ext_substate_autoneg - more information in addition to
- * ETHTOOL_LINK_EXT_STATE_AUTONEG.
- */
+/* More information in addition to ETHTOOL_LINK_EXT_STATE_AUTONEG. */
 enum ethtool_link_ext_substate_autoneg {
 	ETHTOOL_LINK_EXT_SUBSTATE_AN_NO_PARTNER_DETECTED = 1,
 	ETHTOOL_LINK_EXT_SUBSTATE_AN_ACK_NOT_RECEIVED,
@@ -606,9 +613,7 @@ enum ethtool_link_ext_substate_autoneg {
 	ETHTOOL_LINK_EXT_SUBSTATE_AN_NO_HCD,
 };
 
-/**
- * enum ethtool_link_ext_substate_link_training - more information in addition to
- * ETHTOOL_LINK_EXT_STATE_LINK_TRAINING_FAILURE.
+/* More information in addition to ETHTOOL_LINK_EXT_STATE_LINK_TRAINING_FAILURE.
  */
 enum ethtool_link_ext_substate_link_training {
 	ETHTOOL_LINK_EXT_SUBSTATE_LT_KR_FRAME_LOCK_NOT_ACQUIRED = 1,
@@ -617,9 +622,7 @@ enum ethtool_link_ext_substate_link_training {
 	ETHTOOL_LINK_EXT_SUBSTATE_LT_REMOTE_FAULT,
 };
 
-/**
- * enum ethtool_link_ext_substate_logical_mismatch - more information in addition
- * to ETHTOOL_LINK_EXT_STATE_LINK_LOGICAL_MISMATCH.
+/* More information in addition to ETHTOOL_LINK_EXT_STATE_LINK_LOGICAL_MISMATCH.
  */
 enum ethtool_link_ext_substate_link_logical_mismatch {
 	ETHTOOL_LINK_EXT_SUBSTATE_LLM_PCS_DID_NOT_ACQUIRE_BLOCK_LOCK = 1,
@@ -629,19 +632,14 @@ enum ethtool_link_ext_substate_link_logical_mismatch {
 	ETHTOOL_LINK_EXT_SUBSTATE_LLM_RS_FEC_IS_NOT_LOCKED,
 };
 
-/**
- * enum ethtool_link_ext_substate_bad_signal_integrity - more information in
- * addition to ETHTOOL_LINK_EXT_STATE_BAD_SIGNAL_INTEGRITY.
+/* More information in addition to ETHTOOL_LINK_EXT_STATE_BAD_SIGNAL_INTEGRITY.
  */
 enum ethtool_link_ext_substate_bad_signal_integrity {
 	ETHTOOL_LINK_EXT_SUBSTATE_BSI_LARGE_NUMBER_OF_PHYSICAL_ERRORS = 1,
 	ETHTOOL_LINK_EXT_SUBSTATE_BSI_UNSUPPORTED_RATE,
 };
 
-/**
- * enum ethtool_link_ext_substate_cable_issue - more information in
- * addition to ETHTOOL_LINK_EXT_STATE_CABLE_ISSUE.
- */
+/* More information in addition to ETHTOOL_LINK_EXT_STATE_CABLE_ISSUE. */
 enum ethtool_link_ext_substate_cable_issue {
 	ETHTOOL_LINK_EXT_SUBSTATE_CI_UNSUPPORTED_CABLE = 1,
 	ETHTOOL_LINK_EXT_SUBSTATE_CI_CABLE_TEST_FAILURE,
@@ -659,6 +657,7 @@ enum ethtool_link_ext_substate_cable_issue {
  *	now deprecated
  * @ETH_SS_FEATURES: Device feature names
  * @ETH_SS_RSS_HASH_FUNCS: RSS hush function names
+ * @ETH_SS_TUNABLES: tunable names
  * @ETH_SS_PHY_STATS: Statistic names, for use with %ETHTOOL_GPHYSTATS
  * @ETH_SS_PHY_TUNABLES: PHY tunable names
  * @ETH_SS_LINK_MODES: link mode names
@@ -668,6 +667,13 @@ enum ethtool_link_ext_substate_cable_issue {
  * @ETH_SS_TS_TX_TYPES: timestamping Tx types
  * @ETH_SS_TS_RX_FILTERS: timestamping Rx filters
  * @ETH_SS_UDP_TUNNEL_TYPES: UDP tunnel types
+ * @ETH_SS_STATS_STD: standardized stats
+ * @ETH_SS_STATS_ETH_PHY: names of IEEE 802.3 PHY statistics
+ * @ETH_SS_STATS_ETH_MAC: names of IEEE 802.3 MAC statistics
+ * @ETH_SS_STATS_ETH_CTRL: names of IEEE 802.3 MAC Control statistics
+ * @ETH_SS_STATS_RMON: names of RMON statistics
+ *
+ * @ETH_SS_COUNT: number of defined string sets
  */
 enum ethtool_stringset {
 	ETH_SS_TEST		= 0,
@@ -686,6 +692,11 @@ enum ethtool_stringset {
 	ETH_SS_TS_TX_TYPES,
 	ETH_SS_TS_RX_FILTERS,
 	ETH_SS_UDP_TUNNEL_TYPES,
+	ETH_SS_STATS_STD,
+	ETH_SS_STATS_ETH_PHY,
+	ETH_SS_STATS_ETH_MAC,
+	ETH_SS_STATS_ETH_CTRL,
+	ETH_SS_STATS_RMON,
 
 	/* add new constants above here */
 	ETH_SS_COUNT
@@ -713,6 +724,7 @@ struct ethtool_gstrings {
 /**
  * struct ethtool_sset_info - string set information
  * @cmd: Command number = %ETHTOOL_GSSET_INFO
+ * @reserved: Reserved for future use; see the note on reserved space.
  * @sset_mask: On entry, a bitmask of string sets to query, with bits
  *	numbered according to &enum ethtool_stringset.  On return, a
  *	bitmask of those string sets queried that are supported.
@@ -757,6 +769,7 @@ enum ethtool_test_flags {
  * @flags: A bitmask of flags from &enum ethtool_test_flags.  Some
  *	flags may be set by the user on entry; others may be set by
  *	the driver on return.
+ * @reserved: Reserved for future use; see the note on reserved space.
  * @len: On return, the number of test results
  * @data: Array of test results
  *
@@ -957,6 +970,7 @@ union ethtool_flow_union {
  * @vlan_etype: VLAN EtherType
  * @vlan_tci: VLAN tag control information
  * @data: user defined data
+ * @padding: Reserved for future use; see the note on reserved space.
  *
  * Note, @vlan_etype, @vlan_tci, and @data are only valid if %FLOW_EXT
  * is set in &struct ethtool_rx_flow_spec @flow_type.
@@ -1132,7 +1146,8 @@ struct ethtool_rxfh_indir {
  *	hardware hash key.
  * @hfunc: Defines the current RSS hash function used by HW (or to be set to).
  *	Valid values are one of the %ETH_RSS_HASH_*.
- * @rsvd:	Reserved for future extensions.
+ * @rsvd8: Reserved for future use; see the note on reserved space.
+ * @rsvd32: Reserved for future use; see the note on reserved space.
  * @rss_config: RX ring/queue index for each hash value i.e., indirection table
  *	of @indir_size __u32 elements, followed by hash key of @key_size
  *	bytes.
@@ -1300,7 +1315,9 @@ struct ethtool_sfeatures {
  * @so_timestamping: bit mask of the sum of the supported SO_TIMESTAMPING flags
  * @phc_index: device index of the associated PHC, or -1 if there is none
  * @tx_types: bit mask of the supported hwtstamp_tx_types enumeration values
+ * @tx_reserved: Reserved for future use; see the note on reserved space.
  * @rx_filters: bit mask of the supported hwtstamp_rx_filters enumeration values
+ * @rx_reserved: Reserved for future use; see the note on reserved space.
  *
  * The bits in the 'tx_types' and 'rx_filters' fields correspond to
  * the 'hwtstamp_tx_types' and 'hwtstamp_rx_filters' enumeration values,
@@ -1374,15 +1391,33 @@ struct ethtool_per_queue_op {
 };
 
 /**
- * struct ethtool_fecparam - Ethernet forward error correction(fec) parameters
+ * struct ethtool_fecparam - Ethernet Forward Error Correction parameters
  * @cmd: Command number = %ETHTOOL_GFECPARAM or %ETHTOOL_SFECPARAM
- * @active_fec: FEC mode which is active on porte
- * @fec: Bitmask of supported/configured FEC modes
- * @rsvd: Reserved for future extensions. i.e FEC bypass feature.
+ * @active_fec: FEC mode which is active on the port, single bit set, GET only.
+ * @fec: Bitmask of configured FEC modes.
+ * @reserved: Reserved for future extensions, ignore on GET, write 0 for SET.
  *
- * Drivers should reject a non-zero setting of @autoneg when
- * autoneogotiation is disabled (or not supported) for the link.
+ * Note that @reserved was never validated on input and ethtool user space
+ * left it uninitialized when calling SET. Hence going forward it can only be
+ * used to return a value to userspace with GET.
+ *
+ * FEC modes supported by the device can be read via %ETHTOOL_GLINKSETTINGS.
+ * FEC settings are configured by link autonegotiation whenever it's enabled.
+ * With autoneg on %ETHTOOL_GFECPARAM can be used to read the current mode.
+ *
+ * When autoneg is disabled %ETHTOOL_SFECPARAM controls the FEC settings.
+ * It is recommended that drivers only accept a single bit set in @fec.
+ * When multiple bits are set in @fec drivers may pick mode in an implementation
+ * dependent way. Drivers should reject mixing %ETHTOOL_FEC_AUTO_BIT with other
+ * FEC modes, because it's unclear whether in this case other modes constrain
+ * AUTO or are independent choices.
+ * Drivers must reject SET requests if they support none of the requested modes.
+ *
+ * If device does not support FEC drivers may use %ETHTOOL_FEC_NONE instead
+ * of returning %EOPNOTSUPP from %ETHTOOL_GFECPARAM.
  *
+ * See enum ethtool_fec_config_bits for definition of valid bits for both
+ * @fec and @active_fec.
  */
 struct ethtool_fecparam {
 	__u32   cmd;
@@ -1394,11 +1429,16 @@ struct ethtool_fecparam {
 
 /**
  * enum ethtool_fec_config_bits - flags definition of ethtool_fec_configuration
- * @ETHTOOL_FEC_NONE: FEC mode configuration is not supported
- * @ETHTOOL_FEC_AUTO: Default/Best FEC mode provided by driver
- * @ETHTOOL_FEC_OFF: No FEC Mode
- * @ETHTOOL_FEC_RS: Reed-Solomon Forward Error Detection mode
- * @ETHTOOL_FEC_BASER: Base-R/Reed-Solomon Forward Error Detection mode
+ * @ETHTOOL_FEC_NONE_BIT: FEC mode configuration is not supported. Should not
+ *			be used together with other bits. GET only.
+ * @ETHTOOL_FEC_AUTO_BIT: Select default/best FEC mode automatically, usually
+ *			based link mode and SFP parameters read from module's
+ *			EEPROM. This bit does _not_ mean autonegotiation.
+ * @ETHTOOL_FEC_OFF_BIT: No FEC Mode
+ * @ETHTOOL_FEC_RS_BIT: Reed-Solomon FEC Mode
+ * @ETHTOOL_FEC_BASER_BIT: Base-R/Reed-Solomon FEC Mode
+ * @ETHTOOL_FEC_LLRS_BIT: Low Latency Reed Solomon FEC Mode (25G/50G Ethernet
+ *			Consortium)
  */
 enum ethtool_fec_config_bits {
 	ETHTOOL_FEC_NONE_BIT,
@@ -1956,6 +1996,11 @@ enum ethtool_reset_flags {
  *	autonegotiation; 0 if unknown or not applicable.  Read-only.
  * @transceiver: Used to distinguish different possible PHY types,
  *	reported consistently by PHYLIB.  Read-only.
+ * @master_slave_cfg: Master/slave port mode.
+ * @master_slave_state: Master/slave port state.
+ * @reserved: Reserved for future use; see the note on reserved space.
+ * @reserved1: Reserved for future use; see the note on reserved space.
+ * @link_mode_masks: Variable length bitmaps.
  *
  * If autonegotiation is disabled, the speed and @duplex represent the
  * fixed link mode and are writable if the driver supports multiple
diff --git a/uapi/linux/ethtool_netlink.h b/uapi/linux/ethtool_netlink.h
index 0cd6906aa5d5..4653c4c79972 100644
--- a/uapi/linux/ethtool_netlink.h
+++ b/uapi/linux/ethtool_netlink.h
@@ -42,6 +42,10 @@ enum {
 	ETHTOOL_MSG_CABLE_TEST_ACT,
 	ETHTOOL_MSG_CABLE_TEST_TDR_ACT,
 	ETHTOOL_MSG_TUNNEL_INFO_GET,
+	ETHTOOL_MSG_FEC_GET,
+	ETHTOOL_MSG_FEC_SET,
+	ETHTOOL_MSG_MODULE_EEPROM_GET,
+	ETHTOOL_MSG_STATS_GET,
 
 	/* add new constants above here */
 	__ETHTOOL_MSG_USER_CNT,
@@ -80,6 +84,10 @@ enum {
 	ETHTOOL_MSG_CABLE_TEST_NTF,
 	ETHTOOL_MSG_CABLE_TEST_TDR_NTF,
 	ETHTOOL_MSG_TUNNEL_INFO_GET_REPLY,
+	ETHTOOL_MSG_FEC_GET_REPLY,
+	ETHTOOL_MSG_FEC_NTF,
+	ETHTOOL_MSG_MODULE_EEPROM_GET_REPLY,
+	ETHTOOL_MSG_STATS_GET_REPLY,
 
 	/* add new constants above here */
 	__ETHTOOL_MSG_KERNEL_CNT,
@@ -629,6 +637,185 @@ enum {
 	ETHTOOL_A_TUNNEL_INFO_MAX = (__ETHTOOL_A_TUNNEL_INFO_CNT - 1)
 };
 
+/* FEC */
+
+enum {
+	ETHTOOL_A_FEC_UNSPEC,
+	ETHTOOL_A_FEC_HEADER,				/* nest - _A_HEADER_* */
+	ETHTOOL_A_FEC_MODES,				/* bitset */
+	ETHTOOL_A_FEC_AUTO,				/* u8 */
+	ETHTOOL_A_FEC_ACTIVE,				/* u32 */
+	ETHTOOL_A_FEC_STATS,				/* nest - _A_FEC_STAT */
+
+	__ETHTOOL_A_FEC_CNT,
+	ETHTOOL_A_FEC_MAX = (__ETHTOOL_A_FEC_CNT - 1)
+};
+
+enum {
+	ETHTOOL_A_FEC_STAT_UNSPEC,
+	ETHTOOL_A_FEC_STAT_PAD,
+
+	ETHTOOL_A_FEC_STAT_CORRECTED,			/* array, u64 */
+	ETHTOOL_A_FEC_STAT_UNCORR,			/* array, u64 */
+	ETHTOOL_A_FEC_STAT_CORR_BITS,			/* array, u64 */
+
+	/* add new constants above here */
+	__ETHTOOL_A_FEC_STAT_CNT,
+	ETHTOOL_A_FEC_STAT_MAX = (__ETHTOOL_A_FEC_STAT_CNT - 1)
+};
+
+/* MODULE EEPROM */
+
+enum {
+	ETHTOOL_A_MODULE_EEPROM_UNSPEC,
+	ETHTOOL_A_MODULE_EEPROM_HEADER,			/* nest - _A_HEADER_* */
+
+	ETHTOOL_A_MODULE_EEPROM_OFFSET,			/* u32 */
+	ETHTOOL_A_MODULE_EEPROM_LENGTH,			/* u32 */
+	ETHTOOL_A_MODULE_EEPROM_PAGE,			/* u8 */
+	ETHTOOL_A_MODULE_EEPROM_BANK,			/* u8 */
+	ETHTOOL_A_MODULE_EEPROM_I2C_ADDRESS,		/* u8 */
+	ETHTOOL_A_MODULE_EEPROM_DATA,			/* nested */
+
+	__ETHTOOL_A_MODULE_EEPROM_CNT,
+	ETHTOOL_A_MODULE_EEPROM_MAX = (__ETHTOOL_A_MODULE_EEPROM_CNT - 1)
+};
+
+/* STATS */
+
+enum {
+	ETHTOOL_A_STATS_UNSPEC,
+	ETHTOOL_A_STATS_PAD,
+	ETHTOOL_A_STATS_HEADER,			/* nest - _A_HEADER_* */
+	ETHTOOL_A_STATS_GROUPS,			/* bitset */
+
+	ETHTOOL_A_STATS_GRP,			/* nest - _A_STATS_GRP_* */
+
+	/* add new constants above here */
+	__ETHTOOL_A_STATS_CNT,
+	ETHTOOL_A_STATS_MAX = (__ETHTOOL_A_STATS_CNT - 1)
+};
+
+enum {
+	ETHTOOL_STATS_ETH_PHY,
+	ETHTOOL_STATS_ETH_MAC,
+	ETHTOOL_STATS_ETH_CTRL,
+	ETHTOOL_STATS_RMON,
+
+	/* add new constants above here */
+	__ETHTOOL_STATS_CNT
+};
+
+enum {
+	ETHTOOL_A_STATS_GRP_UNSPEC,
+	ETHTOOL_A_STATS_GRP_PAD,
+
+	ETHTOOL_A_STATS_GRP_ID,			/* u32 */
+	ETHTOOL_A_STATS_GRP_SS_ID,		/* u32 */
+
+	ETHTOOL_A_STATS_GRP_STAT,		/* nest */
+
+	ETHTOOL_A_STATS_GRP_HIST_RX,		/* nest */
+	ETHTOOL_A_STATS_GRP_HIST_TX,		/* nest */
+
+	ETHTOOL_A_STATS_GRP_HIST_BKT_LOW,	/* u32 */
+	ETHTOOL_A_STATS_GRP_HIST_BKT_HI,	/* u32 */
+	ETHTOOL_A_STATS_GRP_HIST_VAL,		/* u64 */
+
+	/* add new constants above here */
+	__ETHTOOL_A_STATS_GRP_CNT,
+	ETHTOOL_A_STATS_GRP_MAX = (__ETHTOOL_A_STATS_CNT - 1)
+};
+
+enum {
+	/* 30.3.2.1.5 aSymbolErrorDuringCarrier */
+	ETHTOOL_A_STATS_ETH_PHY_5_SYM_ERR,
+
+	/* add new constants above here */
+	__ETHTOOL_A_STATS_ETH_PHY_CNT,
+	ETHTOOL_A_STATS_ETH_PHY_MAX = (__ETHTOOL_A_STATS_ETH_PHY_CNT - 1)
+};
+
+enum {
+	/* 30.3.1.1.2 aFramesTransmittedOK */
+	ETHTOOL_A_STATS_ETH_MAC_2_TX_PKT,
+	/* 30.3.1.1.3 aSingleCollisionFrames */
+	ETHTOOL_A_STATS_ETH_MAC_3_SINGLE_COL,
+	/* 30.3.1.1.4 aMultipleCollisionFrames */
+	ETHTOOL_A_STATS_ETH_MAC_4_MULTI_COL,
+	/* 30.3.1.1.5 aFramesReceivedOK */
+	ETHTOOL_A_STATS_ETH_MAC_5_RX_PKT,
+	/* 30.3.1.1.6 aFrameCheckSequenceErrors */
+	ETHTOOL_A_STATS_ETH_MAC_6_FCS_ERR,
+	/* 30.3.1.1.7 aAlignmentErrors */
+	ETHTOOL_A_STATS_ETH_MAC_7_ALIGN_ERR,
+	/* 30.3.1.1.8 aOctetsTransmittedOK */
+	ETHTOOL_A_STATS_ETH_MAC_8_TX_BYTES,
+	/* 30.3.1.1.9 aFramesWithDeferredXmissions */
+	ETHTOOL_A_STATS_ETH_MAC_9_TX_DEFER,
+	/* 30.3.1.1.10 aLateCollisions */
+	ETHTOOL_A_STATS_ETH_MAC_10_LATE_COL,
+	/* 30.3.1.1.11 aFramesAbortedDueToXSColls */
+	ETHTOOL_A_STATS_ETH_MAC_11_XS_COL,
+	/* 30.3.1.1.12 aFramesLostDueToIntMACXmitError */
+	ETHTOOL_A_STATS_ETH_MAC_12_TX_INT_ERR,
+	/* 30.3.1.1.13 aCarrierSenseErrors */
+	ETHTOOL_A_STATS_ETH_MAC_13_CS_ERR,
+	/* 30.3.1.1.14 aOctetsReceivedOK */
+	ETHTOOL_A_STATS_ETH_MAC_14_RX_BYTES,
+	/* 30.3.1.1.15 aFramesLostDueToIntMACRcvError */
+	ETHTOOL_A_STATS_ETH_MAC_15_RX_INT_ERR,
+
+	/* 30.3.1.1.18 aMulticastFramesXmittedOK */
+	ETHTOOL_A_STATS_ETH_MAC_18_TX_MCAST,
+	/* 30.3.1.1.19 aBroadcastFramesXmittedOK */
+	ETHTOOL_A_STATS_ETH_MAC_19_TX_BCAST,
+	/* 30.3.1.1.20 aFramesWithExcessiveDeferral */
+	ETHTOOL_A_STATS_ETH_MAC_20_XS_DEFER,
+	/* 30.3.1.1.21 aMulticastFramesReceivedOK */
+	ETHTOOL_A_STATS_ETH_MAC_21_RX_MCAST,
+	/* 30.3.1.1.22 aBroadcastFramesReceivedOK */
+	ETHTOOL_A_STATS_ETH_MAC_22_RX_BCAST,
+	/* 30.3.1.1.23 aInRangeLengthErrors */
+	ETHTOOL_A_STATS_ETH_MAC_23_IR_LEN_ERR,
+	/* 30.3.1.1.24 aOutOfRangeLengthField */
+	ETHTOOL_A_STATS_ETH_MAC_24_OOR_LEN,
+	/* 30.3.1.1.25 aFrameTooLongErrors */
+	ETHTOOL_A_STATS_ETH_MAC_25_TOO_LONG_ERR,
+
+	/* add new constants above here */
+	__ETHTOOL_A_STATS_ETH_MAC_CNT,
+	ETHTOOL_A_STATS_ETH_MAC_MAX = (__ETHTOOL_A_STATS_ETH_MAC_CNT - 1)
+};
+
+enum {
+	/* 30.3.3.3 aMACControlFramesTransmitted */
+	ETHTOOL_A_STATS_ETH_CTRL_3_TX,
+	/* 30.3.3.4 aMACControlFramesReceived */
+	ETHTOOL_A_STATS_ETH_CTRL_4_RX,
+	/* 30.3.3.5 aUnsupportedOpcodesReceived */
+	ETHTOOL_A_STATS_ETH_CTRL_5_RX_UNSUP,
+
+	/* add new constants above here */
+	__ETHTOOL_A_STATS_ETH_CTRL_CNT,
+	ETHTOOL_A_STATS_ETH_CTRL_MAX = (__ETHTOOL_A_STATS_ETH_CTRL_CNT - 1)
+};
+
+enum {
+	/* etherStatsUndersizePkts */
+	ETHTOOL_A_STATS_RMON_UNDERSIZE,
+	/* etherStatsOversizePkts */
+	ETHTOOL_A_STATS_RMON_OVERSIZE,
+	/* etherStatsFragments */
+	ETHTOOL_A_STATS_RMON_FRAG,
+	/* etherStatsJabbers */
+	ETHTOOL_A_STATS_RMON_JABBER,
+
+	/* add new constants above here */
+	__ETHTOOL_A_STATS_RMON_CNT,
+	ETHTOOL_A_STATS_RMON_MAX = (__ETHTOOL_A_STATS_RMON_CNT - 1)
+};
+
 /* generic netlink info */
 #define ETHTOOL_GENL_NAME "ethtool"
 #define ETHTOOL_GENL_VERSION 1
diff --git a/uapi/linux/rtnetlink.h b/uapi/linux/rtnetlink.h
index c66fd247d90a..e01efa281bdc 100644
--- a/uapi/linux/rtnetlink.h
+++ b/uapi/linux/rtnetlink.h
@@ -178,6 +178,13 @@ enum {
 	RTM_GETVLAN,
 #define RTM_GETVLAN	RTM_GETVLAN
 
+	RTM_NEWNEXTHOPBUCKET = 116,
+#define RTM_NEWNEXTHOPBUCKET	RTM_NEWNEXTHOPBUCKET
+	RTM_DELNEXTHOPBUCKET,
+#define RTM_DELNEXTHOPBUCKET	RTM_DELNEXTHOPBUCKET
+	RTM_GETNEXTHOPBUCKET,
+#define RTM_GETNEXTHOPBUCKET	RTM_GETNEXTHOPBUCKET
+
 	__RTM_MAX,
 #define RTM_MAX		(((__RTM_MAX + 3) & ~3) - 1)
 };
@@ -283,6 +290,7 @@ enum {
 #define RTPROT_MROUTED		17	/* Multicast daemon */
 #define RTPROT_KEEPALIVED	18	/* Keepalived daemon */
 #define RTPROT_BABEL		42	/* Babel daemon */
+#define RTPROT_OPENR		99	/* Open Routing (Open/R) Routes */
 #define RTPROT_BGP		186	/* BGP Routes */
 #define RTPROT_ISIS		187	/* ISIS Routes */
 #define RTPROT_OSPF		188	/* OSPF Routes */
@@ -319,6 +327,11 @@ enum rt_scope_t {
 #define RTM_F_FIB_MATCH	        0x2000	/* return full fib lookup match */
 #define RTM_F_OFFLOAD		0x4000	/* route is offloaded */
 #define RTM_F_TRAP		0x8000	/* route is trapping packets */
+#define RTM_F_OFFLOAD_FAILED	0x20000000 /* route offload failed, this value
+					    * is chosen to avoid conflicts with
+					    * other flags defined in
+					    * include/uapi/linux/ipv6_route.h
+					    */
 
 /* Reserved table identifiers */
 
-- 
2.30.2

[PATCH ethtool-next v2 2/7] json: improve array print API

[Jakub Kicinski]

In ethtool when we print an array we usually have a label (non-JSON)
and a key (JSON), because arrays are most often printed entry-per-line
(unlike iproute2 where the output has multiple params on a line
to cater well to multi-interface dumps).

Use this knowledge in the json array API to make it simpler to use.

At the same time (similarly to open_json_object()) do not require
specifying output type. Users can pass an empty string if they
want nothing printed for non-JSON output.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 json_print.c | 20 ++++++++++----------
 json_print.h |  4 ++--
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/json_print.c b/json_print.c
index 56d5b4337e49..4f62767bdbc9 100644
--- a/json_print.c
+++ b/json_print.c
@@ -73,15 +73,15 @@ void close_json_object(void)
 /*
  * Start json array or string array using
  * the provided string as json key (if not null)
- * or as array delimiter in non-json context.
+ * or array delimiter in non-json context.
  */
-void open_json_array(enum output_type type, const char *str)
+void open_json_array(const char *key, const char *str)
 {
-	if (_IS_JSON_CONTEXT(type)) {
-		if (str)
-			jsonw_name(_jw, str);
+	if (is_json_context()) {
+		if (key)
+			jsonw_name(_jw, key);
 		jsonw_start_array(_jw);
-	} else if (_IS_FP_CONTEXT(type)) {
+	} else {
 		printf("%s", str);
 	}
 }
@@ -89,12 +89,12 @@ void open_json_array(enum output_type type, const char *str)
 /*
  * End json array or string array
  */
-void close_json_array(enum output_type type, const char *str)
+void close_json_array(const char *delim)
 {
-	if (_IS_JSON_CONTEXT(type))
+	if (is_json_context())
 		jsonw_end_array(_jw);
-	else if (_IS_FP_CONTEXT(type))
-		printf("%s", str);
+	else
+		printf("%s", delim);
 }
 
 /*
diff --git a/json_print.h b/json_print.h
index cc0c2ea19b59..df15314bafe2 100644
--- a/json_print.h
+++ b/json_print.h
@@ -37,8 +37,8 @@ void fflush_fp(void);
 
 void open_json_object(const char *str);
 void close_json_object(void);
-void open_json_array(enum output_type type, const char *delim);
-void close_json_array(enum output_type type, const char *delim);
+void open_json_array(const char *key, const char *str);
+void close_json_array(const char *delim);
 
 void print_nl(void);
 
-- 
2.30.2

[PATCH ethtool-next v2 3/7] netlink: add FEC support

[Jakub Kicinski]

Add support for FEC get/set over netlink.

Output should be identical but for ordering of RS vs BaseR.
Since the new output is based on link modes RS comes before
BaseR in "Configured FEC encodings". Seems pretty unlikely
someone depends on the ordering there.

Since old API was case insensitive we need to carefully
manage the bit names. Luckily for now most of the modes
are all uppercase.

Beyond that we need to cover up the discrepancy between
the meanings of None and Off. Kernel API uses the link
mode definition, but ethtool needs to maintain the old
interpretation of None meaning "not supported".

Because of those deviations existing link mode parsers
can't be directly reused.

Allocate error code 83 for FEC errors.

JSON support included.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 Makefile.am            |   2 +-
 ethtool.c              |   2 +
 netlink/desc-ethtool.c |  12 ++
 netlink/extapi.h       |   4 +
 netlink/fec.c          | 276 +++++++++++++++++++++++++++++++++++++++++
 netlink/monitor.c      |   4 +
 netlink/netlink.h      |   1 +
 7 files changed, 300 insertions(+), 1 deletion(-)
 create mode 100644 netlink/fec.c

diff --git a/Makefile.am b/Makefile.am
index e3e311d4b274..f643a24af97a 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -35,7 +35,7 @@ ethtool_SOURCES += \
 		  netlink/permaddr.c netlink/prettymsg.c netlink/prettymsg.h \
 		  netlink/features.c netlink/privflags.c netlink/rings.c \
 		  netlink/channels.c netlink/coalesce.c netlink/pause.c \
-		  netlink/eee.c netlink/tsinfo.c \
+		  netlink/eee.c netlink/tsinfo.c netlink/fec.c \
 		  netlink/desc-ethtool.c netlink/desc-genlctrl.c \
 		  netlink/desc-rtnl.c netlink/cable_test.c netlink/tunnels.c \
 		  uapi/linux/ethtool_netlink.h \
diff --git a/ethtool.c b/ethtool.c
index a8339c899cb3..b01a29bcf069 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -5964,11 +5964,13 @@ static const struct option args[] = {
 	{
 		.opts	= "--show-fec",
 		.func	= do_gfec,
+		.nlfunc	= nl_gfec,
 		.help	= "Show FEC settings",
 	},
 	{
 		.opts	= "--set-fec",
 		.func	= do_sfec,
+		.nlfunc	= nl_sfec,
 		.help	= "Set FEC settings",
 		.xhelp	= "		[ encoding auto|off|rs|baser|llrs [...]]\n"
 	},
diff --git a/netlink/desc-ethtool.c b/netlink/desc-ethtool.c
index fe5d7ba796a4..7d14c8b38571 100644
--- a/netlink/desc-ethtool.c
+++ b/netlink/desc-ethtool.c
@@ -318,6 +318,14 @@ const struct pretty_nla_desc __tunnel_info_desc[] = {
 	NLATTR_DESC_NESTED(ETHTOOL_A_TUNNEL_INFO_UDP_PORTS, tunnel_udp),
 };
 
+static const struct pretty_nla_desc __fec_desc[] = {
+	NLATTR_DESC_INVALID(ETHTOOL_A_FEC_UNSPEC),
+	NLATTR_DESC_NESTED(ETHTOOL_A_FEC_HEADER, header),
+	NLATTR_DESC_NESTED(ETHTOOL_A_FEC_MODES, bitset),
+	NLATTR_DESC_BOOL(ETHTOOL_A_FEC_AUTO),
+	NLATTR_DESC_U32(ETHTOOL_A_FEC_ACTIVE),
+};
+
 const struct pretty_nlmsg_desc ethnl_umsg_desc[] = {
 	NLMSG_DESC_INVALID(ETHTOOL_MSG_USER_NONE),
 	NLMSG_DESC(ETHTOOL_MSG_STRSET_GET, strset),
@@ -348,6 +356,8 @@ const struct pretty_nlmsg_desc ethnl_umsg_desc[] = {
 	NLMSG_DESC(ETHTOOL_MSG_CABLE_TEST_ACT, cable_test),
 	NLMSG_DESC(ETHTOOL_MSG_CABLE_TEST_TDR_ACT, cable_test_tdr),
 	NLMSG_DESC(ETHTOOL_MSG_TUNNEL_INFO_GET, tunnel_info),
+	NLMSG_DESC(ETHTOOL_MSG_FEC_GET, fec),
+	NLMSG_DESC(ETHTOOL_MSG_FEC_SET, fec),
 };
 
 const unsigned int ethnl_umsg_n_desc = ARRAY_SIZE(ethnl_umsg_desc);
@@ -383,6 +393,8 @@ const struct pretty_nlmsg_desc ethnl_kmsg_desc[] = {
 	NLMSG_DESC(ETHTOOL_MSG_CABLE_TEST_NTF, cable_test_ntf),
 	NLMSG_DESC(ETHTOOL_MSG_CABLE_TEST_TDR_NTF, cable_test_tdr_ntf),
 	NLMSG_DESC(ETHTOOL_MSG_TUNNEL_INFO_GET_REPLY, tunnel_info),
+	NLMSG_DESC(ETHTOOL_MSG_FEC_GET_REPLY, fec),
+	NLMSG_DESC(ETHTOOL_MSG_FEC_NTF, fec),
 };
 
 const unsigned int ethnl_kmsg_n_desc = ARRAY_SIZE(ethnl_kmsg_desc);
diff --git a/netlink/extapi.h b/netlink/extapi.h
index 761cafb97855..5cadacce08e8 100644
--- a/netlink/extapi.h
+++ b/netlink/extapi.h
@@ -38,6 +38,8 @@ int nl_tsinfo(struct cmd_context *ctx);
 int nl_cable_test(struct cmd_context *ctx);
 int nl_cable_test_tdr(struct cmd_context *ctx);
 int nl_gtunnels(struct cmd_context *ctx);
+int nl_gfec(struct cmd_context *ctx);
+int nl_sfec(struct cmd_context *ctx);
 int nl_monitor(struct cmd_context *ctx);
 
 void nl_monitor_usage(void);
@@ -87,6 +89,8 @@ static inline void nl_monitor_usage(void)
 #define nl_cable_test		NULL
 #define nl_cable_test_tdr	NULL
 #define nl_gtunnels		NULL
+#define nl_gfec			NULL
+#define nl_sfec			NULL
 
 #endif /* ETHTOOL_ENABLE_NETLINK */
 
diff --git a/netlink/fec.c b/netlink/fec.c
new file mode 100644
index 000000000000..bc9e120ce1be
--- /dev/null
+++ b/netlink/fec.c
@@ -0,0 +1,276 @@
+/*
+ * fec.c - netlink implementation of FEC commands
+ *
+ * Implementation of "ethtool --show-fec <dev>" and
+ * "ethtool --set-fec <dev> ..."
+ */
+
+#include <errno.h>
+#include <ctype.h>
+#include <string.h>
+#include <stdio.h>
+
+#include "../internal.h"
+#include "../common.h"
+#include "netlink.h"
+#include "bitset.h"
+#include "parser.h"
+
+/* FEC_GET */
+
+static void
+fec_mode_walk(unsigned int idx, const char *name, bool val, void *data)
+{
+	bool *empty = data;
+
+	if (!val)
+		return;
+	if (empty)
+		*empty = false;
+
+	/* Rename None to Off - in legacy ioctl None means "not supported"
+	 * rather than supported but disabled.
+	 */
+	if (idx == ETHTOOL_LINK_MODE_FEC_NONE_BIT)
+		name = "Off";
+	/* Rename to match the ioctl letter case */
+	else if (idx == ETHTOOL_LINK_MODE_FEC_BASER_BIT)
+		name = "BaseR";
+
+	print_string(PRINT_ANY, NULL, " %s", name);
+}
+
+int fec_reply_cb(const struct nlmsghdr *nlhdr, void *data)
+{
+	const struct nlattr *tb[ETHTOOL_A_FEC_MAX + 1] = {};
+	DECLARE_ATTR_TB_INFO(tb);
+	struct nl_context *nlctx = data;
+	const struct stringset *lm_strings;
+	const char *name;
+	bool fa, empty;
+	bool silent;
+	int err_ret;
+	u32 active;
+	int ret;
+
+	silent = nlctx->is_dump || nlctx->is_monitor;
+	err_ret = silent ? MNL_CB_OK : MNL_CB_ERROR;
+	ret = mnl_attr_parse(nlhdr, GENL_HDRLEN, attr_cb, &tb_info);
+	if (ret < 0)
+		return err_ret;
+	nlctx->devname = get_dev_name(tb[ETHTOOL_A_FEC_HEADER]);
+	if (!dev_ok(nlctx))
+		return err_ret;
+
+	ret = netlink_init_ethnl2_socket(nlctx);
+	if (ret < 0)
+		return err_ret;
+	lm_strings = global_stringset(ETH_SS_LINK_MODES, nlctx->ethnl2_socket);
+
+	active = 0;
+	if (tb[ETHTOOL_A_FEC_ACTIVE])
+		active = mnl_attr_get_u32(tb[ETHTOOL_A_FEC_ACTIVE]);
+
+	if (silent)
+		print_nl();
+
+	open_json_object(NULL);
+
+	print_string(PRINT_ANY, "ifname", "FEC parameters for %s:\n",
+		     nlctx->devname);
+
+	open_json_array("config", "Configured FEC encodings:");
+	fa = tb[ETHTOOL_A_FEC_AUTO] && mnl_attr_get_u8(tb[ETHTOOL_A_FEC_AUTO]);
+	if (fa)
+		print_string(PRINT_ANY, NULL, " %s", "Auto");
+	empty = !fa;
+
+	ret = walk_bitset(tb[ETHTOOL_A_FEC_MODES], lm_strings, fec_mode_walk,
+			  &empty);
+	if (ret < 0)
+		goto err_close_dev;
+	if (empty)
+		print_string(PRINT_ANY, NULL, " %s", "None");
+	close_json_array("\n");
+
+	open_json_array("active", "Active FEC encoding:");
+	if (active) {
+		name = get_string(lm_strings, active);
+		if (name)
+			/* Take care of renames */
+			fec_mode_walk(active, name, true, NULL);
+		else
+			print_uint(PRINT_ANY, NULL, " BIT%u", active);
+	} else {
+		print_string(PRINT_ANY, NULL, " %s", "None");
+	}
+	close_json_array("\n");
+
+	close_json_object();
+
+	return MNL_CB_OK;
+
+err_close_dev:
+	close_json_object();
+	return err_ret;
+}
+
+int nl_gfec(struct cmd_context *ctx)
+{
+	struct nl_context *nlctx = ctx->nlctx;
+	struct nl_socket *nlsk = nlctx->ethnl_socket;
+	int ret;
+
+	if (netlink_cmd_check(ctx, ETHTOOL_MSG_FEC_GET, true))
+		return -EOPNOTSUPP;
+	if (ctx->argc > 0) {
+		fprintf(stderr, "ethtool: unexpected parameter '%s'\n",
+			*ctx->argp);
+		return 1;
+	}
+
+	ret = nlsock_prep_get_request(nlsk, ETHTOOL_MSG_FEC_GET,
+				      ETHTOOL_A_FEC_HEADER, 0);
+	if (ret < 0)
+		return ret;
+
+	new_json_obj(ctx->json);
+	ret = nlsock_send_get_request(nlsk, fec_reply_cb);
+	delete_json_obj();
+	return ret;
+}
+
+/* FEC_SET */
+
+static void strupc(char *dst, const char *src)
+{
+	while (*src)
+		*dst++ = toupper(*src++);
+	*dst = '\0';
+}
+
+static int fec_parse_bitset(struct nl_context *nlctx, uint16_t type,
+			    const void *data __maybe_unused,
+			    struct nl_msg_buff *msgbuff, void *dest)
+{
+	struct nlattr *bitset_attr;
+	struct nlattr *bits_attr;
+	struct nlattr *bit_attr;
+	char upper[ETH_GSTRING_LEN];
+	bool fec_auto = false;
+	int ret;
+
+	if (!type || dest) {
+		fprintf(stderr, "ethtool (%s): internal error parsing '%s'\n",
+			nlctx->cmd, nlctx->param);
+		return -EFAULT;
+	}
+
+	bitset_attr = ethnla_nest_start(msgbuff, type);
+	if (!bitset_attr)
+		return -EMSGSIZE;
+	ret = -EMSGSIZE;
+	if (ethnla_put_flag(msgbuff, ETHTOOL_A_BITSET_NOMASK, true))
+		goto err;
+	bits_attr = ethnla_nest_start(msgbuff, ETHTOOL_A_BITSET_BITS);
+	if (!bits_attr)
+		goto err;
+
+	while (nlctx->argc > 0) {
+		const char *name = *nlctx->argp;
+
+		if (!strcmp(name, "--")) {
+			nlctx->argp++;
+			nlctx->argc--;
+			break;
+		}
+
+		if (!strcasecmp(name, "auto")) {
+			fec_auto = true;
+			goto next;
+		}
+		if (!strcasecmp(name, "off")) {
+			name = "None";
+		} else {
+			strupc(upper, name);
+			name = upper;
+		}
+
+		ret = -EMSGSIZE;
+		bit_attr = ethnla_nest_start(msgbuff,
+					     ETHTOOL_A_BITSET_BITS_BIT);
+		if (!bit_attr)
+			goto err;
+		if (ethnla_put_strz(msgbuff, ETHTOOL_A_BITSET_BIT_NAME, name))
+			goto err;
+		ethnla_nest_end(msgbuff, bit_attr);
+
+next:
+		nlctx->argp++;
+		nlctx->argc--;
+	}
+
+	ethnla_nest_end(msgbuff, bits_attr);
+	ethnla_nest_end(msgbuff, bitset_attr);
+
+	if (ethnla_put_u8(msgbuff, ETHTOOL_A_FEC_AUTO, fec_auto))
+		goto err;
+
+	return 0;
+err:
+	ethnla_nest_cancel(msgbuff, bitset_attr);
+	return ret;
+}
+
+static const struct param_parser sfec_params[] = {
+	{
+		.arg		= "encoding",
+		.type		= ETHTOOL_A_FEC_MODES,
+		.handler	= fec_parse_bitset,
+		.min_argc	= 1,
+	},
+	{}
+};
+
+int nl_sfec(struct cmd_context *ctx)
+{
+	struct nl_context *nlctx = ctx->nlctx;
+	struct nl_msg_buff *msgbuff;
+	struct nl_socket *nlsk;
+	int ret;
+
+	if (netlink_cmd_check(ctx, ETHTOOL_MSG_FEC_SET, false))
+		return -EOPNOTSUPP;
+	if (!ctx->argc) {
+		fprintf(stderr, "ethtool (--set-fec): parameters missing\n");
+		return 1;
+	}
+
+	nlctx->cmd = "--set-fec";
+	nlctx->argp = ctx->argp;
+	nlctx->argc = ctx->argc;
+	nlctx->devname = ctx->devname;
+	nlsk = nlctx->ethnl_socket;
+	msgbuff = &nlsk->msgbuff;
+
+	ret = msg_init(nlctx, msgbuff, ETHTOOL_MSG_FEC_SET,
+		       NLM_F_REQUEST | NLM_F_ACK);
+	if (ret < 0)
+		return 2;
+	if (ethnla_fill_header(msgbuff, ETHTOOL_A_FEC_HEADER,
+			       ctx->devname, 0))
+		return -EMSGSIZE;
+
+	ret = nl_parser(nlctx, sfec_params, NULL, PARSER_GROUP_NONE, NULL);
+	if (ret < 0)
+		return 1;
+
+	ret = nlsock_sendmsg(nlsk, NULL);
+	if (ret < 0)
+		return 83;
+	ret = nlsock_process_reply(nlsk, nomsg_reply_cb, nlctx);
+	if (ret == 0)
+		return 0;
+	else
+		return nlctx->exit_code ?: 83;
+}
diff --git a/netlink/monitor.c b/netlink/monitor.c
index 19f991fce3e4..0c4df9e78ee3 100644
--- a/netlink/monitor.c
+++ b/netlink/monitor.c
@@ -67,6 +67,10 @@ static struct {
 		.cmd	= ETHTOOL_MSG_CABLE_TEST_TDR_NTF,
 		.cb	= cable_test_tdr_ntf_cb,
 	},
+	{
+		.cmd	= ETHTOOL_MSG_FEC_NTF,
+		.cb	= fec_reply_cb,
+	},
 };
 
 static void clear_filter(struct nl_context *nlctx)
diff --git a/netlink/netlink.h b/netlink/netlink.h
index c02558540218..70fa666b20e5 100644
--- a/netlink/netlink.h
+++ b/netlink/netlink.h
@@ -90,6 +90,7 @@ int cable_test_reply_cb(const struct nlmsghdr *nlhdr, void *data);
 int cable_test_ntf_cb(const struct nlmsghdr *nlhdr, void *data);
 int cable_test_tdr_reply_cb(const struct nlmsghdr *nlhdr, void *data);
 int cable_test_tdr_ntf_cb(const struct nlmsghdr *nlhdr, void *data);
+int fec_reply_cb(const struct nlmsghdr *nlhdr, void *data);
 
 /* dump helpers */
 
-- 
2.30.2

[PATCH ethtool-next v2 4/7] netlink: fec: support displaying statistics

[Jakub Kicinski]

Similar implementation-wise to pause stats.
The difference is that stats are reported as arrays
(first entry being total, and remaining ones per-lane).

 # 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
 # ethtool --json -I --show-fec eth0
[ {
        "ifname": "eth0",
        "config": [ "None" ],
        "active": [ "None" ],
        "statistics": {
            "corrected_blocks": {
                "total": 256,
                "lanes": [ 255,1 ]
            },
            "uncorrectable_blocks": {
                "total": 145,
                "lanes": [ 128,17 ]
            }
        }
    } ]

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 netlink/fec.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 84 insertions(+), 1 deletion(-)

diff --git a/netlink/fec.c b/netlink/fec.c
index bc9e120ce1be..f2659199c157 100644
--- a/netlink/fec.c
+++ b/netlink/fec.c
@@ -7,6 +7,7 @@
 
 #include <errno.h>
 #include <ctype.h>
+#include <inttypes.h>
 #include <string.h>
 #include <stdio.h>
 
@@ -40,6 +41,79 @@ fec_mode_walk(unsigned int idx, const char *name, bool val, void *data)
 	print_string(PRINT_ANY, NULL, " %s", name);
 }
 
+static int fec_show_stats(const struct nlattr *nest)
+{
+	const struct nlattr *tb[ETHTOOL_A_FEC_STAT_MAX + 1] = {};
+	DECLARE_ATTR_TB_INFO(tb);
+	static const struct {
+		unsigned int attr;
+		char *name;
+	} stats[] = {
+		{ ETHTOOL_A_FEC_STAT_CORRECTED, "corrected_blocks" },
+		{ ETHTOOL_A_FEC_STAT_UNCORR, "uncorrectable_blocks" },
+		{ ETHTOOL_A_FEC_STAT_CORR_BITS, "corrected_bits" },
+	};
+	bool header = false;
+	unsigned int i;
+	int ret;
+
+	ret = mnl_attr_parse_nested(nest, attr_cb, &tb_info);
+	if (ret < 0)
+		return ret;
+
+	open_json_object("statistics");
+	for (i = 0; i < ARRAY_SIZE(stats); i++) {
+		uint64_t *vals;
+		int lanes, l;
+
+		if (!tb[stats[i].attr] ||
+		    !mnl_attr_get_payload_len(tb[stats[i].attr]))
+			continue;
+
+		if (!header && !is_json_context()) {
+			printf("Statistics:\n");
+			header = true;
+		}
+
+		if (mnl_attr_get_payload_len(tb[stats[i].attr]) % 8) {
+			fprintf(stderr, "malformed netlink message (statistic)\n");
+			goto err_close_stats;
+		}
+
+		vals = mnl_attr_get_payload(tb[stats[i].attr]);
+		lanes = mnl_attr_get_payload_len(tb[stats[i].attr]) / 8 - 1;
+
+		if (!is_json_context()) {
+			fprintf(stdout, "  %s: %" PRIu64 "\n",
+				stats[i].name, *vals++);
+		} else {
+			open_json_object(stats[i].name);
+			print_u64(PRINT_JSON, "total", NULL, *vals++);
+		}
+
+		if (lanes)
+			open_json_array("lanes", "");
+		for (l = 0; l < lanes; l++) {
+			if (!is_json_context())
+				fprintf(stdout, "    Lane %d: %" PRIu64 "\n",
+					l, *vals++);
+			else
+				print_u64(PRINT_JSON, NULL, NULL, *vals++);
+		}
+		if (lanes)
+			close_json_array("");
+
+		close_json_object();
+	}
+	close_json_object();
+
+	return 0;
+
+err_close_stats:
+	close_json_object();
+	return -1;
+}
+
 int fec_reply_cb(const struct nlmsghdr *nlhdr, void *data)
 {
 	const struct nlattr *tb[ETHTOOL_A_FEC_MAX + 1] = {};
@@ -106,6 +180,12 @@ int fec_reply_cb(const struct nlmsghdr *nlhdr, void *data)
 	}
 	close_json_array("\n");
 
+	if (tb[ETHTOOL_A_FEC_STATS]) {
+		ret = fec_show_stats(tb[ETHTOOL_A_FEC_STATS]);
+		if (ret < 0)
+			goto err_close_dev;
+	}
+
 	close_json_object();
 
 	return MNL_CB_OK;
@@ -119,6 +199,7 @@ int nl_gfec(struct cmd_context *ctx)
 {
 	struct nl_context *nlctx = ctx->nlctx;
 	struct nl_socket *nlsk = nlctx->ethnl_socket;
+	u32 flags;
 	int ret;
 
 	if (netlink_cmd_check(ctx, ETHTOOL_MSG_FEC_GET, true))
@@ -129,8 +210,10 @@ int nl_gfec(struct cmd_context *ctx)
 		return 1;
 	}
 
+	flags = get_stats_flag(nlctx, ETHTOOL_MSG_FEC_GET,
+			       ETHTOOL_A_FEC_HEADER);
 	ret = nlsock_prep_get_request(nlsk, ETHTOOL_MSG_FEC_GET,
-				      ETHTOOL_A_FEC_HEADER, 0);
+				      ETHTOOL_A_FEC_HEADER, flags);
 	if (ret < 0)
 		return ret;
 
-- 
2.30.2

[PATCH ethtool-next v2 5/7] ethtool: add nlchk for redirecting to netlink

[Jakub Kicinski]

To support commands which differ from the ioctl implementation
add a new callback which can check if the arguments on the command
line indicate that the request should be sent over netlink.
The decision should be inferred from the arguments, rather
than an explicit --netlink argument.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 ethtool.c         | 5 ++++-
 netlink/extapi.h  | 5 +++--
 netlink/netlink.c | 9 +++++++--
 3 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/ethtool.c b/ethtool.c
index b01a29bcf069..f4a7d396f487 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -5608,6 +5608,7 @@ struct option {
 	const char	*opts;
 	bool		no_dev;
 	int		(*func)(struct cmd_context *);
+	nl_chk_t	nlchk;
 	nl_func_t	nlfunc;
 	const char	*help;
 	const char	*xhelp;
@@ -6270,6 +6271,7 @@ int main(int argc, char **argp)
 	int (*func)(struct cmd_context *);
 	struct cmd_context ctx = {};
 	nl_func_t nlfunc = NULL;
+	nl_chk_t nlchk = NULL;
 	bool no_dev;
 	int ret;
 	int k;
@@ -6329,6 +6331,7 @@ int main(int argc, char **argp)
 		argc--;
 		func = args[k].func;
 		nlfunc = args[k].nlfunc;
+		nlchk = args[k].nlchk;
 		no_dev = args[k].no_dev;
 		goto opt_found;
 	}
@@ -6348,7 +6351,7 @@ int main(int argc, char **argp)
 	}
 	ctx.argc = argc;
 	ctx.argp = argp;
-	netlink_run_handler(&ctx, nlfunc, !func);
+	netlink_run_handler(&ctx, nlchk, nlfunc, !func);
 
 	ret = ioctl_init(&ctx, no_dev);
 	if (ret)
diff --git a/netlink/extapi.h b/netlink/extapi.h
index 5cadacce08e8..d6036a39e920 100644
--- a/netlink/extapi.h
+++ b/netlink/extapi.h
@@ -11,11 +11,12 @@ struct cmd_context;
 struct nl_context;
 
 typedef int (*nl_func_t)(struct cmd_context *);
+typedef bool (*nl_chk_t)(struct cmd_context *);
 
 #ifdef ETHTOOL_ENABLE_NETLINK
 
-void netlink_run_handler(struct cmd_context *ctx, nl_func_t nlfunc,
-			 bool no_fallback);
+void netlink_run_handler(struct cmd_context *ctx, nl_chk_t nlchk,
+			 nl_func_t nlfunc, bool no_fallback);
 
 int nl_gset(struct cmd_context *ctx);
 int nl_sset(struct cmd_context *ctx);
diff --git a/netlink/netlink.c b/netlink/netlink.c
index ffe06339f099..4cee9b23b28f 100644
--- a/netlink/netlink.c
+++ b/netlink/netlink.c
@@ -452,14 +452,15 @@ static void netlink_done(struct cmd_context *ctx)
 /**
  * netlink_run_handler() - run netlink handler for subcommand
  * @ctx:         command context
+ * @nlchk:       netlink capability check
  * @nlfunc:      subcommand netlink handler to call
  * @no_fallback: there is no ioctl fallback handler
  *
  * This function returns only if ioctl() handler should be run as fallback.
  * Otherwise it exits with appropriate return code.
  */
-void netlink_run_handler(struct cmd_context *ctx, nl_func_t nlfunc,
-			 bool no_fallback)
+void netlink_run_handler(struct cmd_context *ctx, nl_chk_t nlchk,
+			 nl_func_t nlfunc, bool no_fallback)
 {
 	bool wildcard = ctx->devname && !strcmp(ctx->devname, WILDCARD_DEVNAME);
 	bool wildcard_unsupported, ioctl_fallback;
@@ -467,6 +468,10 @@ void netlink_run_handler(struct cmd_context *ctx, nl_func_t nlfunc,
 	const char *reason;
 	int ret;
 
+	if (nlchk && !nlchk(ctx)) {
+		reason = "ioctl-only request";
+		goto no_support;
+	}
 	if (ctx->devname && strlen(ctx->devname) >= ALTIFNAMSIZ) {
 		fprintf(stderr, "device name '%s' longer than %u characters\n",
 			ctx->devname, ALTIFNAMSIZ - 1);
-- 
2.30.2

[PATCH ethtool-next v2 6/7] netlink: add support for standard stats

[Jakub Kicinski]

Add support for standard-based stats.

Unlike ethtool -S eth0 the new stats should be allow cross-vendor
compatibility. The interface depends on bitsets and is rather simple.

 # ethtool -S eth0 --groups eth-phy eth-mac rmon
Stats for eth0:
eth-phy-SymbolErrorDuringCarrier: 1
eth-mac-FramesTransmittedOK: 1
eth-mac-FrameTooLongErrors: 1
rmon-etherStatsUndersizePkts: 1
rmon-etherStatsJabbers: 1
rmon-rx-etherStatsPkts64Octets: 1
rmon-rx-etherStatsPkts128to255Octets: 1
rmon-rx-etherStatsPkts1024toMaxOctets: 0

In JSON form stats are grouped and histograms are broken out:

 # ethtool --json -S eth0 --groups eth-phy eth-mac rmon | jq
 [
  {
    "ifname": "eth0",
    "eth-phy": {
      "SymbolErrorDuringCarrier": 1
    },
    "eth-mac": {
      "FramesTransmittedOK": 1,
      "FrameTooLongErrors": 0
    },
    "rmon": {
      "etherStatsUndersizePkts": 1,
      "etherStatsJabbers": 0,
      "rx-pktsNtoM": [
        {
          "low": 0,
          "high": 64,
          "val": 1
        },
        {
          "low": 128,
          "high": 255,
          "val": 1
        },
        {
          "low": 1024,
          "high": 0,
          "val": 0
        }
      ]
    }
  }
 ]

This allows for easy querying, e.g. to add up all packets larger
than 128 (assuming buckets align):

 # ethtool --json -S eth0 --groups eth-phy eth-mac rmon | \
     jq '.[].rmon."rx-pktsNtoM" | map(select(.low >= 128)) | map(.val) | add'
 1

v2: fix hanging "and"

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Tested-by: Ido Schimmel <idosch@nvidia.com>
---
 Makefile.am            |   1 +
 ethtool.8.in           |  20 +++-
 ethtool.c              |   5 +-
 netlink/desc-ethtool.c |  39 ++++++
 netlink/extapi.h       |   4 +
 netlink/stats.c        | 264 +++++++++++++++++++++++++++++++++++++++++
 6 files changed, 330 insertions(+), 3 deletions(-)
 create mode 100644 netlink/stats.c

diff --git a/Makefile.am b/Makefile.am
index f643a24af97a..75c245653cda 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -36,6 +36,7 @@ ethtool_SOURCES += \
 		  netlink/features.c netlink/privflags.c netlink/rings.c \
 		  netlink/channels.c netlink/coalesce.c netlink/pause.c \
 		  netlink/eee.c netlink/tsinfo.c netlink/fec.c \
+		  netlink/stats.c \
 		  netlink/desc-ethtool.c netlink/desc-genlctrl.c \
 		  netlink/desc-rtnl.c netlink/cable_test.c netlink/tunnels.c \
 		  uapi/linux/ethtool_netlink.h \
diff --git a/ethtool.8.in b/ethtool.8.in
index fe49b66888c9..d4e01778d738 100644
--- a/ethtool.8.in
+++ b/ethtool.8.in
@@ -240,6 +240,12 @@ ethtool \- query or control network driver and hardware settings
 .HP
 .B ethtool \-S|\-\-statistics
 .I devname
+.RB [\fB\-\-groups
+.RB [\fBeth\-phy\fP]
+.RB [\fBeth\-mac\fP]
+.RB [\fBeth\-ctrl\fP]
+.RN [\fBrmon\fP]
+.RB ]
 .HP
 .B ethtool \-\-phy\-statistics
 .I devname
@@ -652,8 +658,18 @@ Restarts auto-negotiation on the specified Ethernet device, if
 auto-negotiation is enabled.
 .TP
 .B \-S \-\-statistics
-Queries the specified network device for NIC- and driver-specific
-statistics.
+Queries the specified network device for standard (IEEE, IETF, etc.), or NIC-
+and driver-specific statistics. NIC- and driver-specific statistics are
+requested when no group of statistics is specified.
+
+NIC- and driver-specific statistics and standard statistics are independent,
+devices may implement either, both or none. There is little commonality between
+naming of NIC- and driver-specific statistics across vendors.
+.RS 4
+.TP
+.B \fB\-\-groups [\fBeth\-phy\fP] [\fBeth\-mac\fP] [\fBeth\-ctrl\fP] [\fBrmon\fP]
+Request groups of standard device statistics.
+.RE
 .TP
 .B \-\-phy\-statistics
 Queries the specified network device for PHY specific statistics.
diff --git a/ethtool.c b/ethtool.c
index f4a7d396f487..b23fb05a3016 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -5761,7 +5761,10 @@ static const struct option args[] = {
 	{
 		.opts	= "-S|--statistics",
 		.func	= do_gnicstats,
-		.help	= "Show adapter statistics"
+		.nlchk	= nl_gstats_chk,
+		.nlfunc	= nl_gstats,
+		.help	= "Show adapter statistics",
+		.xhelp	= "               [ --groups [eth-phy] [eth-mac] [eth-ctrl] [rmon] ]\n"
 	},
 	{
 		.opts	= "--phy-statistics",
diff --git a/netlink/desc-ethtool.c b/netlink/desc-ethtool.c
index 7d14c8b38571..8ea7c53a7a5f 100644
--- a/netlink/desc-ethtool.c
+++ b/netlink/desc-ethtool.c
@@ -326,6 +326,43 @@ static const struct pretty_nla_desc __fec_desc[] = {
 	NLATTR_DESC_U32(ETHTOOL_A_FEC_ACTIVE),
 };
 
+static const struct pretty_nla_desc __stats_grp_stat_desc[] = {
+	NLATTR_DESC_U64(0),  NLATTR_DESC_U64(1),  NLATTR_DESC_U64(2),
+	NLATTR_DESC_U64(3),  NLATTR_DESC_U64(4),  NLATTR_DESC_U64(5),
+	NLATTR_DESC_U64(6),  NLATTR_DESC_U64(7),  NLATTR_DESC_U64(8),
+	NLATTR_DESC_U64(9),  NLATTR_DESC_U64(10), NLATTR_DESC_U64(11),
+	NLATTR_DESC_U64(12), NLATTR_DESC_U64(13), NLATTR_DESC_U64(14),
+	NLATTR_DESC_U64(15), NLATTR_DESC_U64(16), NLATTR_DESC_U64(17),
+	NLATTR_DESC_U64(18), NLATTR_DESC_U64(19), NLATTR_DESC_U64(20),
+	NLATTR_DESC_U64(21), NLATTR_DESC_U64(22), NLATTR_DESC_U64(23),
+	NLATTR_DESC_U64(24), NLATTR_DESC_U64(25), NLATTR_DESC_U64(26),
+	NLATTR_DESC_U64(27), NLATTR_DESC_U64(28), NLATTR_DESC_U64(29),
+};
+
+static const struct pretty_nla_desc __stats_grp_hist_desc[] = {
+	NLATTR_DESC_U32(ETHTOOL_A_STATS_GRP_HIST_BKT_LOW),
+	NLATTR_DESC_U32(ETHTOOL_A_STATS_GRP_HIST_BKT_HI),
+	NLATTR_DESC_U64(ETHTOOL_A_STATS_GRP_HIST_VAL),
+};
+
+static const struct pretty_nla_desc __stats_grp_desc[] = {
+	NLATTR_DESC_INVALID(ETHTOOL_A_STATS_GRP_UNSPEC),
+	NLATTR_DESC_INVALID(ETHTOOL_A_STATS_GRP_PAD),
+	NLATTR_DESC_U32(ETHTOOL_A_STATS_GRP_ID),
+	NLATTR_DESC_U32(ETHTOOL_A_STATS_GRP_SS_ID),
+	NLATTR_DESC_NESTED(ETHTOOL_A_STATS_GRP_STAT, stats_grp_stat),
+	NLATTR_DESC_NESTED(ETHTOOL_A_STATS_GRP_HIST_RX, stats_grp_hist),
+	NLATTR_DESC_NESTED(ETHTOOL_A_STATS_GRP_HIST_TX, stats_grp_hist),
+};
+
+static const struct pretty_nla_desc __stats_desc[] = {
+	NLATTR_DESC_INVALID(ETHTOOL_A_STATS_UNSPEC),
+	NLATTR_DESC_INVALID(ETHTOOL_A_STATS_PAD),
+	NLATTR_DESC_NESTED(ETHTOOL_A_STATS_HEADER, header),
+	NLATTR_DESC_NESTED(ETHTOOL_A_STATS_GROUPS, bitset),
+	NLATTR_DESC_NESTED(ETHTOOL_A_STATS_GRP, stats_grp),
+};
+
 const struct pretty_nlmsg_desc ethnl_umsg_desc[] = {
 	NLMSG_DESC_INVALID(ETHTOOL_MSG_USER_NONE),
 	NLMSG_DESC(ETHTOOL_MSG_STRSET_GET, strset),
@@ -358,6 +395,7 @@ const struct pretty_nlmsg_desc ethnl_umsg_desc[] = {
 	NLMSG_DESC(ETHTOOL_MSG_TUNNEL_INFO_GET, tunnel_info),
 	NLMSG_DESC(ETHTOOL_MSG_FEC_GET, fec),
 	NLMSG_DESC(ETHTOOL_MSG_FEC_SET, fec),
+	NLMSG_DESC(ETHTOOL_MSG_STATS_GET, stats),
 };
 
 const unsigned int ethnl_umsg_n_desc = ARRAY_SIZE(ethnl_umsg_desc);
@@ -395,6 +433,7 @@ const struct pretty_nlmsg_desc ethnl_kmsg_desc[] = {
 	NLMSG_DESC(ETHTOOL_MSG_TUNNEL_INFO_GET_REPLY, tunnel_info),
 	NLMSG_DESC(ETHTOOL_MSG_FEC_GET_REPLY, fec),
 	NLMSG_DESC(ETHTOOL_MSG_FEC_NTF, fec),
+	NLMSG_DESC(ETHTOOL_MSG_STATS_GET_REPLY, stats),
 };
 
 const unsigned int ethnl_kmsg_n_desc = ARRAY_SIZE(ethnl_kmsg_desc);
diff --git a/netlink/extapi.h b/netlink/extapi.h
index d6036a39e920..97113eb98ca0 100644
--- a/netlink/extapi.h
+++ b/netlink/extapi.h
@@ -41,6 +41,8 @@ int nl_cable_test_tdr(struct cmd_context *ctx);
 int nl_gtunnels(struct cmd_context *ctx);
 int nl_gfec(struct cmd_context *ctx);
 int nl_sfec(struct cmd_context *ctx);
+bool nl_gstats_chk(struct cmd_context *ctx);
+int nl_gstats(struct cmd_context *ctx);
 int nl_monitor(struct cmd_context *ctx);
 
 void nl_monitor_usage(void);
@@ -92,6 +94,8 @@ static inline void nl_monitor_usage(void)
 #define nl_gtunnels		NULL
 #define nl_gfec			NULL
 #define nl_sfec			NULL
+#define nl_gstats_chk		NULL
+#define nl_gstats		NULL
 
 #endif /* ETHTOOL_ENABLE_NETLINK */
 
diff --git a/netlink/stats.c b/netlink/stats.c
new file mode 100644
index 000000000000..e3ca58b0010c
--- /dev/null
+++ b/netlink/stats.c
@@ -0,0 +1,264 @@
+/*
+ * stats.c - netlink implementation of stats
+ *
+ * Implementation of "ethtool -S <dev> [--groups <types>] etc."
+ */
+
+#include <errno.h>
+#include <ctype.h>
+#include <inttypes.h>
+#include <string.h>
+#include <stdio.h>
+
+#include "../internal.h"
+#include "../common.h"
+#include "netlink.h"
+#include "parser.h"
+#include "strset.h"
+
+static int parse_rmon_hist_one(const char *grp_name, const struct nlattr *hist,
+			       const char *dir)
+{
+	const struct nlattr *tb[ETHTOOL_A_STATS_GRP_HIST_VAL + 1] = {};
+	DECLARE_ATTR_TB_INFO(tb);
+	unsigned long long val;
+	unsigned int low, hi;
+	int ret;
+
+	ret = mnl_attr_parse_nested(hist, attr_cb, &tb_info);
+	if (ret < 0) {
+		fprintf(stderr, "invalid kernel response - malformed histogram entry\n");
+		return 1;
+	}
+
+	if (!tb[ETHTOOL_A_STATS_GRP_HIST_BKT_LOW] ||
+	    !tb[ETHTOOL_A_STATS_GRP_HIST_BKT_HI] ||
+	    !tb[ETHTOOL_A_STATS_GRP_HIST_VAL]) {
+		fprintf(stderr, "invalid kernel response - histogram entry missing attributes\n");
+		return 1;
+	}
+
+	low = mnl_attr_get_u32(tb[ETHTOOL_A_STATS_GRP_HIST_BKT_LOW]);
+	hi = mnl_attr_get_u32(tb[ETHTOOL_A_STATS_GRP_HIST_BKT_HI]);
+	val = mnl_attr_get_u64(tb[ETHTOOL_A_STATS_GRP_HIST_VAL]);
+
+	if (!is_json_context()) {
+		fprintf(stdout, "%s-%s-etherStatsPkts", dir, grp_name);
+
+		if (low && hi) {
+			fprintf(stdout, "%uto%uOctets: %llu\n", low, hi, val);
+		} else if (hi) {
+			fprintf(stdout, "%uOctets: %llu\n", hi, val);
+		} else if (low) {
+			fprintf(stdout, "%utoMaxOctets: %llu\n", low, val);
+		} else {
+			fprintf(stderr, "invalid kernel response - bad histogram entry bounds\n");
+			return 1;
+		}
+	} else {
+		open_json_object(NULL);
+		print_uint(PRINT_JSON, "low", NULL, low);
+		print_uint(PRINT_JSON, "high", NULL, hi);
+		print_u64(PRINT_JSON, "val", NULL, val);
+		close_json_object();
+	}
+
+	return 0;
+}
+
+static int parse_rmon_hist(const struct nlattr *grp, const char *grp_name,
+			   const char *name, const char *dir, unsigned int type)
+{
+	const struct nlattr *attr;
+
+	open_json_array(name, "");
+
+	mnl_attr_for_each_nested(attr, grp) {
+		if (mnl_attr_get_type(attr) == type &&
+		    parse_rmon_hist_one(grp_name, attr, dir))
+			goto err_close_rmon;
+	}
+	close_json_array("");
+
+	return 0;
+
+err_close_rmon:
+	close_json_array("");
+	return 1;
+}
+
+static int parse_grp(struct nl_context *nlctx, const struct nlattr *grp,
+		     const struct stringset *std_str)
+{
+	const struct nlattr *tb[ETHTOOL_A_STATS_GRP_SS_ID + 1] = {};
+	DECLARE_ATTR_TB_INFO(tb);
+	bool hist_rx = false, hist_tx = false;
+	const struct stringset *stat_str;
+	const struct nlattr *attr, *stat;
+	const char *std_name, *name;
+	unsigned int ss_id, id, s;
+	unsigned long long val;
+	int ret;
+
+	ret = mnl_attr_parse_nested(grp, attr_cb, &tb_info);
+	if (ret < 0)
+		return 1;
+
+	if (!tb[ETHTOOL_A_STATS_GRP_ID])
+		return 1;
+	if (!tb[ETHTOOL_A_STATS_GRP_SS_ID])
+		return 0;
+
+	id = mnl_attr_get_u32(tb[ETHTOOL_A_STATS_GRP_ID]);
+	ss_id = mnl_attr_get_u32(tb[ETHTOOL_A_STATS_GRP_SS_ID]);
+
+	stat_str = global_stringset(ss_id, nlctx->ethnl2_socket);
+
+	std_name = get_string(std_str, id);
+	open_json_object(std_name);
+
+	mnl_attr_for_each_nested(attr, grp) {
+		switch (mnl_attr_get_type(attr)) {
+		case ETHTOOL_A_STATS_GRP_STAT:
+			break;
+		case ETHTOOL_A_STATS_GRP_HIST_RX:
+			hist_rx = true;
+			continue;
+		case ETHTOOL_A_STATS_GRP_HIST_TX:
+			hist_tx = true;
+			continue;
+		default:
+			continue;
+		}
+
+		stat = mnl_attr_get_payload(attr);
+		ret = mnl_attr_validate(stat, MNL_TYPE_U64);
+		if (ret) {
+			fprintf(stderr, "invalid kernel response - bad statistic entry\n");
+			goto err_close_grp;
+		}
+		s = mnl_attr_get_type(stat);
+		name = get_string(stat_str, s);
+		if (!name || !name[0])
+			continue;
+
+		if (!is_json_context())
+			fprintf(stdout, "%s-%s: ", std_name, name);
+
+		val = mnl_attr_get_u64(stat);
+		print_u64(PRINT_ANY, name, "%llu\n", val);
+	}
+
+	if (hist_rx)
+		parse_rmon_hist(grp, std_name, "rx-pktsNtoM", "rx",
+				ETHTOOL_A_STATS_GRP_HIST_RX);
+	if (hist_tx)
+		parse_rmon_hist(grp, std_name, "tx-pktsNtoM", "tx",
+				ETHTOOL_A_STATS_GRP_HIST_TX);
+
+	close_json_object();
+
+	return 0;
+
+err_close_grp:
+	close_json_object();
+	return 1;
+}
+
+static int stats_reply_cb(const struct nlmsghdr *nlhdr, void *data)
+{
+	const struct nlattr *tb[ETHTOOL_A_STATS_MAX + 1] = {};
+	DECLARE_ATTR_TB_INFO(tb);
+	struct nl_context *nlctx = data;
+	const struct stringset *std_str;
+	const struct nlattr *attr;
+	bool silent;
+	int err_ret;
+	int ret;
+
+	silent = nlctx->is_dump || nlctx->is_monitor;
+	err_ret = silent ? MNL_CB_OK : MNL_CB_ERROR;
+	ret = mnl_attr_parse(nlhdr, GENL_HDRLEN, attr_cb, &tb_info);
+	if (ret < 0)
+		return err_ret;
+	nlctx->devname = get_dev_name(tb[ETHTOOL_A_STATS_HEADER]);
+	if (!dev_ok(nlctx))
+		return err_ret;
+
+	ret = netlink_init_ethnl2_socket(nlctx);
+	if (ret < 0)
+		return err_ret;
+	std_str = global_stringset(ETH_SS_STATS_STD, nlctx->ethnl2_socket);
+
+	if (silent)
+		print_nl();
+
+	open_json_object(NULL);
+
+	print_string(PRINT_ANY, "ifname", "Standard stats for %s:\n",
+		     nlctx->devname);
+
+	mnl_attr_for_each(attr, nlhdr, GENL_HDRLEN) {
+		if (mnl_attr_get_type(attr) == ETHTOOL_A_STATS_GRP) {
+			ret = parse_grp(nlctx, attr, std_str);
+			if (ret)
+				goto err_close_dev;
+		}
+	}
+
+	close_json_object();
+
+	return MNL_CB_OK;
+
+err_close_dev:
+	close_json_object();
+	return err_ret;
+}
+
+static const struct bitset_parser_data stats_parser_data = {
+	.no_mask	= true,
+	.force_hex	= false,
+};
+
+static const struct param_parser stats_params[] = {
+	{
+		.arg		= "--groups",
+		.type		= ETHTOOL_A_STATS_GROUPS,
+		.handler	= nl_parse_bitset,
+		.handler_data	= &stats_parser_data,
+		.min_argc	= 1,
+	},
+	{}
+};
+
+int nl_gstats(struct cmd_context *ctx)
+{
+	struct nl_context *nlctx = ctx->nlctx;
+	struct nl_socket *nlsk = nlctx->ethnl_socket;
+	int ret;
+
+	ret = nlsock_prep_get_request(nlsk, ETHTOOL_MSG_STATS_GET,
+				      ETHTOOL_A_STATS_HEADER, 0);
+	if (ret < 0)
+		return ret;
+
+	nlctx->cmd = "-S";
+	nlctx->argp = ctx->argp;
+	nlctx->argc = ctx->argc;
+	nlctx->devname = ctx->devname;
+	nlsk = nlctx->ethnl_socket;
+
+	ret = nl_parser(nlctx, stats_params, NULL, PARSER_GROUP_NONE, NULL);
+	if (ret < 0)
+		return 1;
+
+	new_json_obj(ctx->json);
+	ret = nlsock_send_get_request(nlsk, stats_reply_cb);
+	delete_json_obj();
+	return ret;
+}
+
+bool nl_gstats_chk(struct cmd_context *ctx)
+{
+	return ctx->argc;
+}
-- 
2.30.2

[PATCH ethtool-next v2 7/7] netlink: stats: add on --all-groups option

[Jakub Kicinski]

Add a switch for querying all statistic groups available
in the kernel.

To reject --groups and --all-groups being specified
for one request add a concept of "parameter equivalency"
in the parser. Alternative of having a special group
type like "--groups all" seems less clean.

Suggested-by: Ido Schimmel <idosch@nvidia.com>
Tested-by: Ido Schimmel <idosch@nvidia.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 ethtool.8.in     |  5 ++++-
 ethtool.c        |  2 +-
 netlink/parser.c | 17 ++++++++++++++-
 netlink/parser.h |  4 ++++
 netlink/stats.c  | 55 ++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 80 insertions(+), 3 deletions(-)

diff --git a/ethtool.8.in b/ethtool.8.in
index d4e01778d738..652e98af0384 100644
--- a/ethtool.8.in
+++ b/ethtool.8.in
@@ -240,7 +240,7 @@ ethtool \- query or control network driver and hardware settings
 .HP
 .B ethtool \-S|\-\-statistics
 .I devname
-.RB [\fB\-\-groups
+.RB [\fB\-\-all\-groups\fP|\fB\-\-groups
 .RB [\fBeth\-phy\fP]
 .RB [\fBeth\-mac\fP]
 .RB [\fBeth\-ctrl\fP]
@@ -667,6 +667,9 @@ devices may implement either, both or none. There is little commonality between
 naming of NIC- and driver-specific statistics across vendors.
 .RS 4
 .TP
+.B \fB\-\-all\-groups
+.E
+.TP
 .B \fB\-\-groups [\fBeth\-phy\fP] [\fBeth\-mac\fP] [\fBeth\-ctrl\fP] [\fBrmon\fP]
 Request groups of standard device statistics.
 .RE
diff --git a/ethtool.c b/ethtool.c
index b23fb05a3016..34ea64229244 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -5764,7 +5764,7 @@ static const struct option args[] = {
 		.nlchk	= nl_gstats_chk,
 		.nlfunc	= nl_gstats,
 		.help	= "Show adapter statistics",
-		.xhelp	= "               [ --groups [eth-phy] [eth-mac] [eth-ctrl] [rmon] ]\n"
+		.xhelp	= "               [ --all-groups | --groups [eth-phy] [eth-mac] [eth-ctrl] [rmon] ]\n"
 	},
 	{
 		.opts	= "--phy-statistics",
diff --git a/netlink/parser.c b/netlink/parser.c
index c2eae93efb69..d15fa332cc55 100644
--- a/netlink/parser.c
+++ b/netlink/parser.c
@@ -919,6 +919,17 @@ static void __parser_set(uint64_t *map, unsigned int idx)
 	map[idx / 64] |= (1 << (idx % 64));
 }
 
+static void __parser_set_group(const struct param_parser *params,
+			       uint64_t *map, unsigned int equiv_group)
+{
+	const struct param_parser *parser;
+	unsigned int idx = 0;
+
+	for (parser = params; parser->arg; parser++, idx++)
+		if (parser->equiv_group == equiv_group)
+			__parser_set(map, idx);
+}
+
 struct tmp_buff {
 	struct nl_msg_buff	*msgbuff;
 	unsigned int		id;
@@ -1074,7 +1085,11 @@ int nl_parser(struct nl_context *nlctx, const struct param_parser *params,
 			parser_err_min_argc(nlctx, parser->min_argc);
 			goto out_free;
 		}
-		__parser_set(params_seen, parser - params);
+		if (parser->equiv_group)
+			__parser_set_group(params, params_seen,
+					   parser->equiv_group);
+		else
+			__parser_set(params_seen, parser - params);
 
 		buff = NULL;
 		if (parser->group)
diff --git a/netlink/parser.h b/netlink/parser.h
index 28f26ccc2a1c..57ad6d0498a8 100644
--- a/netlink/parser.h
+++ b/netlink/parser.h
@@ -43,6 +43,10 @@ struct param_parser {
 	unsigned int		min_argc;
 	/* if @dest is passed to nl_parser(), offset to store value */
 	unsigned int		dest_offset;
+	/* parameter equivalency group - only one parameter from a group
+	 * can be specified, 0 means no group
+	 */
+	unsigned int		equiv_group;
 };
 
 /* data structures used for handler data */
diff --git a/netlink/stats.c b/netlink/stats.c
index e3ca58b0010c..0a2569c349a4 100644
--- a/netlink/stats.c
+++ b/netlink/stats.c
@@ -220,6 +220,54 @@ static const struct bitset_parser_data stats_parser_data = {
 	.force_hex	= false,
 };
 
+static int stats_parse_all_groups(struct nl_context *nlctx, uint16_t type,
+				  const void *data, struct nl_msg_buff *msgbuff,
+				  void *dest)
+{
+	const struct stringset *std_str;
+	struct nlattr *nest;
+	int i, ret, nbits;
+	uint32_t *bits;
+
+	if (data || dest)
+		return -EFAULT;
+
+	/* ethnl2 and strset code already does caching */
+	ret = netlink_init_ethnl2_socket(nlctx);
+	if (ret < 0)
+		return ret;
+	std_str = global_stringset(ETH_SS_STATS_STD, nlctx->ethnl2_socket);
+
+	nbits = get_count(std_str);
+	bits = calloc(DIV_ROUND_UP(nbits, 32), sizeof(uint32_t));
+	if (!bits)
+		return -ENOMEM;
+
+	for (i = 0; i < nbits; i++)
+		bits[i / 32] |= 1U << (i % 32);
+
+	ret = -EMSGSIZE;
+	nest = ethnla_nest_start(msgbuff, type);
+	if (!nest)
+		goto err_free;
+
+	if (ethnla_put_flag(msgbuff, ETHTOOL_A_BITSET_NOMASK, true) ||
+	    ethnla_put_u32(msgbuff, ETHTOOL_A_BITSET_SIZE, nbits) ||
+	    ethnla_put(msgbuff, ETHTOOL_A_BITSET_VALUE,
+		       DIV_ROUND_UP(nbits, 32) * sizeof(uint32_t), bits))
+		goto err_cancel;
+
+	ethnla_nest_end(msgbuff, nest);
+	free(bits);
+	return 0;
+
+err_cancel:
+	ethnla_nest_cancel(msgbuff, nest);
+err_free:
+	free(bits);
+	return ret;
+}
+
 static const struct param_parser stats_params[] = {
 	{
 		.arg		= "--groups",
@@ -227,6 +275,13 @@ static const struct param_parser stats_params[] = {
 		.handler	= nl_parse_bitset,
 		.handler_data	= &stats_parser_data,
 		.min_argc	= 1,
+		.equiv_group	= 1,
+	},
+	{
+		.arg		= "--all-groups",
+		.type		= ETHTOOL_A_STATS_GROUPS,
+		.handler	= stats_parse_all_groups,
+		.equiv_group	= 1,
 	},
 	{}
 };
-- 
2.30.2

Re: [PATCH ethtool-next v2 1/7] update UAPI header copies

[Michal Kubecek]

[-- Attachment #1: Type: text/plain, Size: 827 bytes --]

On Thu, Apr 22, 2021 at 08:40:44AM -0700, Jakub Kicinski wrote:
> Update to kernel commit 6ecaf81d4ac6.
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
>  uapi/linux/ethtool.h         | 109 ++++++++++++++------
>  uapi/linux/ethtool_netlink.h | 187 +++++++++++++++++++++++++++++++++++
>  uapi/linux/rtnetlink.h       |  13 +++
>  3 files changed, 277 insertions(+), 32 deletions(-)

There should be also a change in uapi/linux/if_link.h (diff below).

Michal

diff --git a/uapi/linux/if_link.h b/uapi/linux/if_link.h
index c96880c51c93..50193377e5e4 100644
--- a/uapi/linux/if_link.h
+++ b/uapi/linux/if_link.h
@@ -809,7 +809,6 @@ enum {
 	IFLA_GTP_FD1,
 	IFLA_GTP_PDP_HASHSIZE,
 	IFLA_GTP_ROLE,
-	IFLA_GTP_COLLECT_METADATA,
 	__IFLA_GTP_MAX,
 };
 #define IFLA_GTP_MAX (__IFLA_GTP_MAX - 1)

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH ethtool-next v2 7/7] netlink: stats: add on --all-groups option

[Michal Kubecek]

[-- Attachment #1: Type: text/plain, Size: 514 bytes --]

On Thu, Apr 22, 2021 at 08:40:50AM -0700, Jakub Kicinski wrote:
> Subject: [PATCH ethtool-next v2 7/7] netlink: stats: add on --all-groups option

"on" should be "an" here?

> Add a switch for querying all statistic groups available
> in the kernel.
> 
> To reject --groups and --all-groups being specified
> for one request add a concept of "parameter equivalency"

I like the idea but the term "equivalency" may be a bit misleading.
Maybe "alternatives" would express the relation better.

Michal

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH ethtool-next v2 5/7] ethtool: add nlchk for redirecting to netlink

[Michal Kubecek]

[-- Attachment #1: Type: text/plain, Size: 643 bytes --]

On Thu, Apr 22, 2021 at 08:40:48AM -0700, Jakub Kicinski wrote:
> To support commands which differ from the ioctl implementation
> add a new callback which can check if the arguments on the command
> line indicate that the request should be sent over netlink.
> The decision should be inferred from the arguments, rather
> than an explicit --netlink argument.
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>

This breaks build with --disable-netlink, the !ETHTOOL_ENABLE_NETLINK
version of netlink_run_handler() needs to be updated as well.

Other than this and the two minor nitpicks, the series looks good to me.

Michal

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH ethtool-next v2 7/7] netlink: stats: add on --all-groups option

[Jakub Kicinski]

On Sun, 2 May 2021 23:36:40 +0200 Michal Kubecek wrote:
> > Add a switch for querying all statistic groups available
> > in the kernel.
> > 
> > To reject --groups and --all-groups being specified
> > for one request add a concept of "parameter equivalency"  
> 
> I like the idea but the term "equivalency" may be a bit misleading.
> Maybe "alternatives" would express the relation better.

Thanks for the review! I take it you mean to rename the code as well?
Not just the commit message?

Re: [PATCH ethtool-next v2 7/7] netlink: stats: add on --all-groups option

[Michal Kubecek]

On Mon, May 03, 2021 at 08:59:01AM -0700, Jakub Kicinski wrote:
> On Sun, 2 May 2021 23:36:40 +0200 Michal Kubecek wrote:
> > > Add a switch for querying all statistic groups available
> > > in the kernel.
> > > 
> > > To reject --groups and --all-groups being specified
> > > for one request add a concept of "parameter equivalency"  
> > 
> > I like the idea but the term "equivalency" may be a bit misleading.
> > Maybe "alternatives" would express the relation better.
> 
> Thanks for the review! I take it you mean to rename the code as well?
> Not just the commit message?

Yes, if we agree that the term is a bit misleading, I would rather see
the struct member renamed.

Michal