All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next v4 0/8] Support setting lanes via ethtool
@ 2021-02-02 18:06 Danielle Ratson
  2021-02-02 18:06 ` [PATCH net-next v4 1/8] ethtool: Validate master slave configuration before rtnl_lock() Danielle Ratson
                   ` (9 more replies)
  0 siblings, 10 replies; 14+ messages in thread
From: Danielle Ratson @ 2021-02-02 18:06 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, jiri, andrew, f.fainelli, mkubecek, mlxsw, idosch,
	Danielle Ratson

Some speeds can be achieved with different number of lanes. For example,
100Gbps can be achieved using two lanes of 50Gbps or four lanes of
25Gbps. This patchset adds a new selector that allows ethtool to
advertise link modes according to their number of lanes and also force a
specific number of lanes when autonegotiation is off.

Advertising all link modes with a speed of 100Gbps that use two lanes:

$ ethtool -s swp1 speed 100000 lanes 2 autoneg on

Forcing a speed of 100Gbps using four lanes:

$ ethtool -s swp1 speed 100000 lanes 4 autoneg off

Patchset overview:

Patch #1-#2 allows user space to configure the desired number of lanes.

Patch #3-#4 adjusts ethtool to dump to user space the number of lanes
currently in use.

Patches #5-#7 add support for lanes configuration in mlxsw.

Patch #8 adds a selftest.

v4:
	* Add patch #1 for validating parameters before rtnl_lock().

v3:
	* Patch #1: Change ethtool_ops.capabilities to be a bitfield,
	  and set min and max for the lanes policy.
	* Patch #2: Remove LINK_MODE_UNKNOWN and move the speed, duplex
	  and lanes derivation to the wrapper
	  __ethtool_get_link_ksettings().
	* Patch #5: Set the bitfield of supporting lanes in the driver
	* to 'true'.
	* Patch #7: Move the test to drivers/net/mlxsw.

v2:
	* Patch #1: Remove ETHTOOL_LANES defines and simply use a number
	  instead.
	* Patches #2,#6: Pass link mode from driver to ethtool instead
	* of the parameters themselves.
	* Patch #5: Add an actual width field for spectrum-2 link modes
	  in order to set the suitable link mode when lanes parameter is
	  passed.
	* Patch #6: Changed lanes to be unsigned in
	  'struct link_mode_info'.
	* Patch #7: Remove the test for recieving max_width when lanes
	  is not set by user. When not setting lanes, we don't promise
	  anything regarding what number of lanes will be chosen.

Danielle Ratson (8):
  ethtool: Validate master slave configuration before rtnl_lock()
  ethtool: Extend link modes settings uAPI with lanes
  ethtool: Get link mode in use instead of speed and duplex parameters
  ethtool: Expose the number of lanes in use
  mlxsw: ethtool: Remove max lanes filtering
  mlxsw: ethtool: Add support for setting lanes when autoneg is off
  mlxsw: ethtool: Pass link mode in use to ethtool
  net: selftests: Add lanes setting test

 Documentation/networking/ethtool-netlink.rst  |  11 +-
 .../net/ethernet/mellanox/mlxsw/spectrum.h    |  13 +-
 .../mellanox/mlxsw/spectrum_ethtool.c         | 196 ++++++++++-------
 include/linux/ethtool.h                       |   5 +
 include/uapi/linux/ethtool_netlink.h          |   1 +
 net/ethtool/common.c                          | 147 +++++++++++++
 net/ethtool/common.h                          |   7 +
 net/ethtool/ioctl.c                           |  18 +-
 net/ethtool/linkmodes.c                       | 208 ++++++------------
 net/ethtool/netlink.h                         |   2 +-
 .../drivers/net/mlxsw/ethtool_lanes.sh        | 187 ++++++++++++++++
 .../selftests/net/forwarding/ethtool_lib.sh   |  34 +++
 tools/testing/selftests/net/forwarding/lib.sh |  28 +++
 13 files changed, 626 insertions(+), 231 deletions(-)
 create mode 100755 tools/testing/selftests/drivers/net/mlxsw/ethtool_lanes.sh

-- 
2.26.2


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

* [PATCH net-next v4 1/8] ethtool: Validate master slave configuration before rtnl_lock()
  2021-02-02 18:06 [PATCH net-next v4 0/8] Support setting lanes via ethtool Danielle Ratson
@ 2021-02-02 18:06 ` Danielle Ratson
  2021-02-02 18:06 ` [PATCH net-next v4 2/8] ethtool: Extend link modes settings uAPI with lanes Danielle Ratson
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Danielle Ratson @ 2021-02-02 18:06 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, jiri, andrew, f.fainelli, mkubecek, mlxsw, idosch,
	Danielle Ratson

Create a new function for input validations to be called before
rtnl_lock() and move the master slave validation to that function.

This would be a cleanup for next patch that would add another validation
to the new function.

Signed-off-by: Danielle Ratson <danieller@nvidia.com>
---
 net/ethtool/linkmodes.c | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/net/ethtool/linkmodes.c b/net/ethtool/linkmodes.c
index c5bcb9abc8b9..bb8a3351fb72 100644
--- a/net/ethtool/linkmodes.c
+++ b/net/ethtool/linkmodes.c
@@ -325,6 +325,21 @@ static bool ethnl_validate_master_slave_cfg(u8 cfg)
 	return false;
 }
 
+static int ethnl_check_linkmodes(struct genl_info *info, struct nlattr **tb)
+{
+	const struct nlattr *master_slave_cfg;
+
+	master_slave_cfg = tb[ETHTOOL_A_LINKMODES_MASTER_SLAVE_CFG];
+	if (master_slave_cfg &&
+	    !ethnl_validate_master_slave_cfg(nla_get_u8(master_slave_cfg))) {
+		NL_SET_ERR_MSG_ATTR(info->extack, master_slave_cfg,
+				    "master/slave value is invalid");
+		return -EOPNOTSUPP;
+	}
+
+	return 0;
+}
+
 static int ethnl_update_linkmodes(struct genl_info *info, struct nlattr **tb,
 				  struct ethtool_link_ksettings *ksettings,
 				  bool *mod)
@@ -336,19 +351,11 @@ static int ethnl_update_linkmodes(struct genl_info *info, struct nlattr **tb,
 
 	master_slave_cfg = tb[ETHTOOL_A_LINKMODES_MASTER_SLAVE_CFG];
 	if (master_slave_cfg) {
-		u8 cfg = nla_get_u8(master_slave_cfg);
-
 		if (lsettings->master_slave_cfg == MASTER_SLAVE_CFG_UNSUPPORTED) {
 			NL_SET_ERR_MSG_ATTR(info->extack, master_slave_cfg,
 					    "master/slave configuration not supported by device");
 			return -EOPNOTSUPP;
 		}
-
-		if (!ethnl_validate_master_slave_cfg(cfg)) {
-			NL_SET_ERR_MSG_ATTR(info->extack, master_slave_cfg,
-					    "master/slave value is invalid");
-			return -EOPNOTSUPP;
-		}
 	}
 
 	*mod = false;
@@ -386,6 +393,10 @@ int ethnl_set_linkmodes(struct sk_buff *skb, struct genl_info *info)
 	bool mod = false;
 	int ret;
 
+	ret = ethnl_check_linkmodes(info, tb);
+	if (ret < 0)
+		return ret;
+
 	ret = ethnl_parse_header_dev_get(&req_info,
 					 tb[ETHTOOL_A_LINKMODES_HEADER],
 					 genl_info_net(info), info->extack,
-- 
2.26.2


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

* [PATCH net-next v4 2/8] ethtool: Extend link modes settings uAPI with lanes
  2021-02-02 18:06 [PATCH net-next v4 0/8] Support setting lanes via ethtool Danielle Ratson
  2021-02-02 18:06 ` [PATCH net-next v4 1/8] ethtool: Validate master slave configuration before rtnl_lock() Danielle Ratson
@ 2021-02-02 18:06 ` Danielle Ratson
  2021-02-02 18:06 ` [PATCH net-next v4 3/8] ethtool: Get link mode in use instead of speed and duplex parameters Danielle Ratson
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Danielle Ratson @ 2021-02-02 18:06 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, jiri, andrew, f.fainelli, mkubecek, mlxsw, idosch,
	Danielle Ratson

Currently, when auto negotiation is on, the user can advertise all the
linkmodes which correspond to a specific speed, but does not have a
similar selector for the number of lanes. This is significant when a
specific speed can be achieved using different number of lanes.  For
example, 2x50 or 4x25.

Add 'ETHTOOL_A_LINKMODES_LANES' attribute and expand 'struct
ethtool_link_settings' with lanes field in order to implement a new
lanes-selector that will enable the user to advertise a specific number
of lanes as well.

When auto negotiation is off, lanes parameter can be forced only if the
driver supports it. Add a capability bit in 'struct ethtool_ops' that
allows ethtool know if the driver can handle the lanes parameter when
auto negotiation is off, so if it does not, an error message will be
returned when trying to set lanes.

Example:

$ ethtool -s swp1 lanes 4
$ ethtool swp1
  Settings for swp1:
	Supported ports: [ FIBRE ]
        Supported link modes:   1000baseKX/Full
                                10000baseKR/Full
                                40000baseCR4/Full
				40000baseSR4/Full
				40000baseLR4/Full
                                25000baseCR/Full
                                25000baseSR/Full
				50000baseCR2/Full
                                100000baseSR4/Full
				100000baseCR4/Full
        Supported pause frame use: Symmetric Receive-only
        Supports auto-negotiation: Yes
        Supported FEC modes: Not reported
        Advertised link modes:  40000baseCR4/Full
				40000baseSR4/Full
				40000baseLR4/Full
                                100000baseSR4/Full
				100000baseCR4/Full
        Advertised pause frame use: No
        Advertised auto-negotiation: Yes
        Advertised FEC modes: Not reported
        Speed: Unknown!
        Duplex: Unknown! (255)
        Auto-negotiation: on
        Port: Direct Attach Copper
        PHYAD: 0
        Transceiver: internal
        Link detected: no

Signed-off-by: Danielle Ratson <danieller@nvidia.com>
---

Notes:
    v4:
    	* Change the way specifying lanes param in link_mode_params.
    	* Remove ETHTOOL_LANES_UNKNOWN uAPI constant and don't report
    	  nlattr if the parameter is unknown.
    	* Make lanes u8 in link_mode_info.
    	* Add nlattr for lanes_cfg and move one lanes validation to the new
    	  function from the last patch.
    
    v3:
    	* Change ethtool_ops.capabilities to be a bitfield and rename it.
    	* Add an according kdoc.
    	* Set min and max for the lanes policy.
    	* Change the lanes validation to be the power of two, now that the
    	  min and max are set.
    
    v2:
    	* Remove ETHTOOL_LANES defines and simply use a number instead.

 Documentation/networking/ethtool-netlink.rst | 11 ++-
 include/linux/ethtool.h                      |  4 +
 include/uapi/linux/ethtool_netlink.h         |  1 +
 net/ethtool/linkmodes.c                      | 96 +++++++++++++++++---
 net/ethtool/netlink.h                        |  2 +-
 5 files changed, 93 insertions(+), 21 deletions(-)

diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst
index 30b98245979f..05073482db05 100644
--- a/Documentation/networking/ethtool-netlink.rst
+++ b/Documentation/networking/ethtool-netlink.rst
@@ -431,16 +431,17 @@ Request contents:
   ``ETHTOOL_A_LINKMODES_SPEED``               u32     link speed (Mb/s)
   ``ETHTOOL_A_LINKMODES_DUPLEX``              u8      duplex mode
   ``ETHTOOL_A_LINKMODES_MASTER_SLAVE_CFG``    u8      Master/slave port mode
+  ``ETHTOOL_A_LINKMODES_LANES``               u32     lanes
   ==========================================  ======  ==========================
 
 ``ETHTOOL_A_LINKMODES_OURS`` bit set allows setting advertised link modes. If
 autonegotiation is on (either set now or kept from before), advertised modes
 are not changed (no ``ETHTOOL_A_LINKMODES_OURS`` attribute) and at least one
-of speed and duplex is specified, kernel adjusts advertised modes to all
-supported modes matching speed, duplex or both (whatever is specified). This
-autoselection is done on ethtool side with ioctl interface, netlink interface
-is supposed to allow requesting changes without knowing what exactly kernel
-supports.
+of speed, duplex and lanes is specified, kernel adjusts advertised modes to all
+supported modes matching speed, duplex, lanes or all (whatever is specified).
+This autoselection is done on ethtool side with ioctl interface, netlink
+interface is supposed to allow requesting changes without knowing what exactly
+kernel supports.
 
 
 LINKSTATE_GET
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index e3da25b51ae4..1ab13c5dfb2f 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -128,6 +128,7 @@ struct ethtool_link_ksettings {
 		__ETHTOOL_DECLARE_LINK_MODE_MASK(advertising);
 		__ETHTOOL_DECLARE_LINK_MODE_MASK(lp_advertising);
 	} link_modes;
+	u32	lanes;
 };
 
 /**
@@ -265,6 +266,8 @@ struct ethtool_pause_stats {
 
 /**
  * struct ethtool_ops - optional netdev operations
+ * @cap_link_lanes_supported: indicates if the driver supports lanes
+ *	parameter.
  * @supported_coalesce_params: supported types of interrupt coalescing.
  * @get_drvinfo: Report driver/device information.  Should only set the
  *	@driver, @version, @fw_version and @bus_info fields.  If not
@@ -420,6 +423,7 @@ struct ethtool_pause_stats {
  * of the generic netdev features interface.
  */
 struct ethtool_ops {
+	u32     cap_link_lanes_supported:1;
 	u32	supported_coalesce_params;
 	void	(*get_drvinfo)(struct net_device *, struct ethtool_drvinfo *);
 	int	(*get_regs_len)(struct net_device *);
diff --git a/include/uapi/linux/ethtool_netlink.h b/include/uapi/linux/ethtool_netlink.h
index e2bf36e6964b..a286635ac9b8 100644
--- a/include/uapi/linux/ethtool_netlink.h
+++ b/include/uapi/linux/ethtool_netlink.h
@@ -227,6 +227,7 @@ enum {
 	ETHTOOL_A_LINKMODES_DUPLEX,		/* u8 */
 	ETHTOOL_A_LINKMODES_MASTER_SLAVE_CFG,	/* u8 */
 	ETHTOOL_A_LINKMODES_MASTER_SLAVE_STATE,	/* u8 */
+	ETHTOOL_A_LINKMODES_LANES,		/* u32 */
 
 	/* add new constants above here */
 	__ETHTOOL_A_LINKMODES_CNT,
diff --git a/net/ethtool/linkmodes.c b/net/ethtool/linkmodes.c
index bb8a3351fb72..db3e31fc6709 100644
--- a/net/ethtool/linkmodes.c
+++ b/net/ethtool/linkmodes.c
@@ -152,12 +152,47 @@ const struct ethnl_request_ops ethnl_linkmodes_request_ops = {
 
 struct link_mode_info {
 	int				speed;
+	u8				lanes;
 	u8				duplex;
 };
 
-#define __DEFINE_LINK_MODE_PARAMS(_speed, _type, _duplex) \
-	[ETHTOOL_LINK_MODE(_speed, _type, _duplex)] = { \
-		.speed	= SPEED_ ## _speed, \
+#define __LINK_MODE_LANES_CR		1
+#define __LINK_MODE_LANES_CR2		2
+#define __LINK_MODE_LANES_CR4		4
+#define __LINK_MODE_LANES_CR8		8
+#define __LINK_MODE_LANES_DR		1
+#define __LINK_MODE_LANES_DR2		2
+#define __LINK_MODE_LANES_DR4		4
+#define __LINK_MODE_LANES_DR8		8
+#define __LINK_MODE_LANES_KR		1
+#define __LINK_MODE_LANES_KR2		2
+#define __LINK_MODE_LANES_KR4		4
+#define __LINK_MODE_LANES_KR8		8
+#define __LINK_MODE_LANES_SR		1
+#define __LINK_MODE_LANES_SR2		2
+#define __LINK_MODE_LANES_SR4		4
+#define __LINK_MODE_LANES_SR8		8
+#define __LINK_MODE_LANES_ER		1
+#define __LINK_MODE_LANES_KX		1
+#define __LINK_MODE_LANES_KX4		4
+#define __LINK_MODE_LANES_LR		1
+#define __LINK_MODE_LANES_LR4		4
+#define __LINK_MODE_LANES_LR4_ER4	4
+#define __LINK_MODE_LANES_LR_ER_FR	1
+#define __LINK_MODE_LANES_LR2_ER2_FR2	2
+#define __LINK_MODE_LANES_LR4_ER4_FR4	4
+#define __LINK_MODE_LANES_LR8_ER8_FR8	8
+#define __LINK_MODE_LANES_LRM		1
+#define __LINK_MODE_LANES_MLD2		2
+#define __LINK_MODE_LANES_T		1
+#define __LINK_MODE_LANES_T1		1
+#define __LINK_MODE_LANES_X		1
+#define __LINK_MODE_LANES_FX		1
+
+#define __DEFINE_LINK_MODE_PARAMS(_speed, _type, _duplex)	\
+	[ETHTOOL_LINK_MODE(_speed, _type, _duplex)] = {		\
+		.speed  = SPEED_ ## _speed, \
+		.lanes  = __LINK_MODE_LANES_ ## _type, \
 		.duplex	= __DUPLEX_ ## _duplex \
 	}
 #define __DUPLEX_Half DUPLEX_HALF
@@ -165,6 +200,7 @@ struct link_mode_info {
 #define __DEFINE_SPECIAL_MODE_PARAMS(_mode) \
 	[ETHTOOL_LINK_MODE_ ## _mode ## _BIT] = { \
 		.speed	= SPEED_UNKNOWN, \
+		.lanes	= 0, \
 		.duplex	= DUPLEX_UNKNOWN, \
 	}
 
@@ -274,16 +310,17 @@ const struct nla_policy ethnl_linkmodes_set_policy[] = {
 	[ETHTOOL_A_LINKMODES_SPEED]		= { .type = NLA_U32 },
 	[ETHTOOL_A_LINKMODES_DUPLEX]		= { .type = NLA_U8 },
 	[ETHTOOL_A_LINKMODES_MASTER_SLAVE_CFG]	= { .type = NLA_U8 },
+	[ETHTOOL_A_LINKMODES_LANES]		= NLA_POLICY_RANGE(NLA_U32, 1, 8),
 };
 
-/* Set advertised link modes to all supported modes matching requested speed
- * and duplex values. Called when autonegotiation is on, speed or duplex is
- * requested but no link mode change. This is done in userspace with ioctl()
- * interface, move it into kernel for netlink.
+/* Set advertised link modes to all supported modes matching requested speed,
+ * lanes and duplex values. Called when autonegotiation is on, speed, lanes or
+ * duplex is requested but no link mode change. This is done in userspace with
+ * ioctl() interface, move it into kernel for netlink.
  * Returns true if advertised modes bitmap was modified.
  */
 static bool ethnl_auto_linkmodes(struct ethtool_link_ksettings *ksettings,
-				 bool req_speed, bool req_duplex)
+				 bool req_speed, bool req_lanes, bool req_duplex)
 {
 	unsigned long *advertising = ksettings->link_modes.advertising;
 	unsigned long *supported = ksettings->link_modes.supported;
@@ -302,6 +339,7 @@ static bool ethnl_auto_linkmodes(struct ethtool_link_ksettings *ksettings,
 			continue;
 		if (test_bit(i, supported) &&
 		    (!req_speed || info->speed == ksettings->base.speed) &&
+		    (!req_lanes || info->lanes == ksettings->lanes) &&
 		    (!req_duplex || info->duplex == ksettings->base.duplex))
 			set_bit(i, advertising);
 		else
@@ -327,7 +365,7 @@ static bool ethnl_validate_master_slave_cfg(u8 cfg)
 
 static int ethnl_check_linkmodes(struct genl_info *info, struct nlattr **tb)
 {
-	const struct nlattr *master_slave_cfg;
+	const struct nlattr *master_slave_cfg, *lanes_cfg;
 
 	master_slave_cfg = tb[ETHTOOL_A_LINKMODES_MASTER_SLAVE_CFG];
 	if (master_slave_cfg &&
@@ -337,16 +375,23 @@ static int ethnl_check_linkmodes(struct genl_info *info, struct nlattr **tb)
 		return -EOPNOTSUPP;
 	}
 
+	lanes_cfg = tb[ETHTOOL_A_LINKMODES_LANES];
+	if (lanes_cfg && !is_power_of_2(nla_get_u32(lanes_cfg))) {
+		NL_SET_ERR_MSG_ATTR(info->extack, lanes_cfg,
+				    "lanes value is invalid");
+		return -EINVAL;
+	}
+
 	return 0;
 }
 
 static int ethnl_update_linkmodes(struct genl_info *info, struct nlattr **tb,
 				  struct ethtool_link_ksettings *ksettings,
-				  bool *mod)
+				  bool *mod, const struct net_device *dev)
 {
 	struct ethtool_link_settings *lsettings = &ksettings->base;
-	bool req_speed, req_duplex;
-	const struct nlattr *master_slave_cfg;
+	bool req_speed, req_lanes, req_duplex;
+	const struct nlattr *master_slave_cfg, *lanes_cfg;
 	int ret;
 
 	master_slave_cfg = tb[ETHTOOL_A_LINKMODES_MASTER_SLAVE_CFG];
@@ -360,10 +405,30 @@ static int ethnl_update_linkmodes(struct genl_info *info, struct nlattr **tb,
 
 	*mod = false;
 	req_speed = tb[ETHTOOL_A_LINKMODES_SPEED];
+	req_lanes = tb[ETHTOOL_A_LINKMODES_LANES];
 	req_duplex = tb[ETHTOOL_A_LINKMODES_DUPLEX];
 
 	ethnl_update_u8(&lsettings->autoneg, tb[ETHTOOL_A_LINKMODES_AUTONEG],
 			mod);
+
+	lanes_cfg = tb[ETHTOOL_A_LINKMODES_LANES];
+	if (lanes_cfg) {
+		/* If autoneg is off and lanes parameter is not supported by the
+		 * driver, return an error.
+		 */
+		if (!lsettings->autoneg &&
+		    !dev->ethtool_ops->cap_link_lanes_supported) {
+			NL_SET_ERR_MSG_ATTR(info->extack, lanes_cfg,
+					    "lanes configuration not supported by device");
+			return -EOPNOTSUPP;
+		}
+	} else if (!lsettings->autoneg) {
+		/* If autoneg is off and lanes parameter is not passed from user,
+		 * set the lanes parameter to 0.
+		 */
+		ksettings->lanes = 0;
+	}
+
 	ret = ethnl_update_bitset(ksettings->link_modes.advertising,
 				  __ETHTOOL_LINK_MODE_MASK_NBITS,
 				  tb[ETHTOOL_A_LINKMODES_OURS], link_mode_names,
@@ -372,13 +437,14 @@ static int ethnl_update_linkmodes(struct genl_info *info, struct nlattr **tb,
 		return ret;
 	ethnl_update_u32(&lsettings->speed, tb[ETHTOOL_A_LINKMODES_SPEED],
 			 mod);
+	ethnl_update_u32(&ksettings->lanes, lanes_cfg, mod);
 	ethnl_update_u8(&lsettings->duplex, tb[ETHTOOL_A_LINKMODES_DUPLEX],
 			mod);
 	ethnl_update_u8(&lsettings->master_slave_cfg, master_slave_cfg, mod);
 
 	if (!tb[ETHTOOL_A_LINKMODES_OURS] && lsettings->autoneg &&
-	    (req_speed || req_duplex) &&
-	    ethnl_auto_linkmodes(ksettings, req_speed, req_duplex))
+	    (req_speed || req_lanes || req_duplex) &&
+	    ethnl_auto_linkmodes(ksettings, req_speed, req_lanes, req_duplex))
 		*mod = true;
 
 	return 0;
@@ -420,7 +486,7 @@ int ethnl_set_linkmodes(struct sk_buff *skb, struct genl_info *info)
 		goto out_ops;
 	}
 
-	ret = ethnl_update_linkmodes(info, tb, &ksettings, &mod);
+	ret = ethnl_update_linkmodes(info, tb, &ksettings, &mod, dev);
 	if (ret < 0)
 		goto out_ops;
 
diff --git a/net/ethtool/netlink.h b/net/ethtool/netlink.h
index d8efec516d86..6eabd58d81bf 100644
--- a/net/ethtool/netlink.h
+++ b/net/ethtool/netlink.h
@@ -351,7 +351,7 @@ extern const struct nla_policy ethnl_strset_get_policy[ETHTOOL_A_STRSET_COUNTS_O
 extern const struct nla_policy ethnl_linkinfo_get_policy[ETHTOOL_A_LINKINFO_HEADER + 1];
 extern const struct nla_policy ethnl_linkinfo_set_policy[ETHTOOL_A_LINKINFO_TP_MDIX_CTRL + 1];
 extern const struct nla_policy ethnl_linkmodes_get_policy[ETHTOOL_A_LINKMODES_HEADER + 1];
-extern const struct nla_policy ethnl_linkmodes_set_policy[ETHTOOL_A_LINKMODES_MASTER_SLAVE_CFG + 1];
+extern const struct nla_policy ethnl_linkmodes_set_policy[ETHTOOL_A_LINKMODES_LANES + 1];
 extern const struct nla_policy ethnl_linkstate_get_policy[ETHTOOL_A_LINKSTATE_HEADER + 1];
 extern const struct nla_policy ethnl_debug_get_policy[ETHTOOL_A_DEBUG_HEADER + 1];
 extern const struct nla_policy ethnl_debug_set_policy[ETHTOOL_A_DEBUG_MSGMASK + 1];
-- 
2.26.2


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

* [PATCH net-next v4 3/8] ethtool: Get link mode in use instead of speed and duplex parameters
  2021-02-02 18:06 [PATCH net-next v4 0/8] Support setting lanes via ethtool Danielle Ratson
  2021-02-02 18:06 ` [PATCH net-next v4 1/8] ethtool: Validate master slave configuration before rtnl_lock() Danielle Ratson
  2021-02-02 18:06 ` [PATCH net-next v4 2/8] ethtool: Extend link modes settings uAPI with lanes Danielle Ratson
@ 2021-02-02 18:06 ` Danielle Ratson
  2021-02-22  9:41   ` Eric Dumazet
  2021-02-02 18:06 ` [PATCH net-next v4 4/8] ethtool: Expose the number of lanes in use Danielle Ratson
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 14+ messages in thread
From: Danielle Ratson @ 2021-02-02 18:06 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, jiri, andrew, f.fainelli, mkubecek, mlxsw, idosch,
	Danielle Ratson

Currently, when user space queries the link's parameters, as speed and
duplex, each parameter is passed from the driver to ethtool.

Instead, get the link mode bit in use, and derive each of the parameters
from it in ethtool.

Signed-off-by: Danielle Ratson <danieller@nvidia.com>
---

Notes:
    v3:
    	* Remove 'ETHTOOL_A_LINKMODES_LINK_MODE' from Documentation since
    	  it is not used.
    	* Remove LINK_MODE_UNKNOWN from uapi.
    	* Remove an unnecessary loop.
    	* Move link_mode_info and link_mode_params to common file.
    	* Move the speed, duplex and lanes derivation to the wrapper
    	  __ethtool_get_link_ksettings().

 include/linux/ethtool.h |   1 +
 net/ethtool/common.c    | 147 +++++++++++++++++++++++++++++++++++++
 net/ethtool/common.h    |   7 ++
 net/ethtool/ioctl.c     |  18 ++++-
 net/ethtool/linkmodes.c | 157 +---------------------------------------
 5 files changed, 174 insertions(+), 156 deletions(-)

diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 1ab13c5dfb2f..ec4cd3921c67 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -129,6 +129,7 @@ struct ethtool_link_ksettings {
 		__ETHTOOL_DECLARE_LINK_MODE_MASK(lp_advertising);
 	} link_modes;
 	u32	lanes;
+	enum ethtool_link_mode_bit_indices link_mode;
 };
 
 /**
diff --git a/net/ethtool/common.c b/net/ethtool/common.c
index 181220101a6e..835b9bba3e7e 100644
--- a/net/ethtool/common.c
+++ b/net/ethtool/common.c
@@ -198,6 +198,153 @@ const char link_mode_names[][ETH_GSTRING_LEN] = {
 };
 static_assert(ARRAY_SIZE(link_mode_names) == __ETHTOOL_LINK_MODE_MASK_NBITS);
 
+#define __LINK_MODE_LANES_CR		1
+#define __LINK_MODE_LANES_CR2		2
+#define __LINK_MODE_LANES_CR4		4
+#define __LINK_MODE_LANES_CR8		8
+#define __LINK_MODE_LANES_DR		1
+#define __LINK_MODE_LANES_DR2		2
+#define __LINK_MODE_LANES_DR4		4
+#define __LINK_MODE_LANES_DR8		8
+#define __LINK_MODE_LANES_KR		1
+#define __LINK_MODE_LANES_KR2		2
+#define __LINK_MODE_LANES_KR4		4
+#define __LINK_MODE_LANES_KR8		8
+#define __LINK_MODE_LANES_SR		1
+#define __LINK_MODE_LANES_SR2		2
+#define __LINK_MODE_LANES_SR4		4
+#define __LINK_MODE_LANES_SR8		8
+#define __LINK_MODE_LANES_ER		1
+#define __LINK_MODE_LANES_KX		1
+#define __LINK_MODE_LANES_KX4		4
+#define __LINK_MODE_LANES_LR		1
+#define __LINK_MODE_LANES_LR4		4
+#define __LINK_MODE_LANES_LR4_ER4	4
+#define __LINK_MODE_LANES_LR_ER_FR	1
+#define __LINK_MODE_LANES_LR2_ER2_FR2	2
+#define __LINK_MODE_LANES_LR4_ER4_FR4	4
+#define __LINK_MODE_LANES_LR8_ER8_FR8	8
+#define __LINK_MODE_LANES_LRM		1
+#define __LINK_MODE_LANES_MLD2		2
+#define __LINK_MODE_LANES_T		1
+#define __LINK_MODE_LANES_T1		1
+#define __LINK_MODE_LANES_X		1
+#define __LINK_MODE_LANES_FX		1
+
+#define __DEFINE_LINK_MODE_PARAMS(_speed, _type, _duplex)	\
+	[ETHTOOL_LINK_MODE(_speed, _type, _duplex)] = {		\
+		.speed  = SPEED_ ## _speed, \
+		.lanes  = __LINK_MODE_LANES_ ## _type, \
+		.duplex	= __DUPLEX_ ## _duplex \
+	}
+#define __DUPLEX_Half DUPLEX_HALF
+#define __DUPLEX_Full DUPLEX_FULL
+#define __DEFINE_SPECIAL_MODE_PARAMS(_mode) \
+	[ETHTOOL_LINK_MODE_ ## _mode ## _BIT] = { \
+		.speed	= SPEED_UNKNOWN, \
+		.lanes	= 0, \
+		.duplex	= DUPLEX_UNKNOWN, \
+	}
+
+const struct link_mode_info link_mode_params[] = {
+	__DEFINE_LINK_MODE_PARAMS(10, T, Half),
+	__DEFINE_LINK_MODE_PARAMS(10, T, Full),
+	__DEFINE_LINK_MODE_PARAMS(100, T, Half),
+	__DEFINE_LINK_MODE_PARAMS(100, T, Full),
+	__DEFINE_LINK_MODE_PARAMS(1000, T, Half),
+	__DEFINE_LINK_MODE_PARAMS(1000, T, Full),
+	__DEFINE_SPECIAL_MODE_PARAMS(Autoneg),
+	__DEFINE_SPECIAL_MODE_PARAMS(TP),
+	__DEFINE_SPECIAL_MODE_PARAMS(AUI),
+	__DEFINE_SPECIAL_MODE_PARAMS(MII),
+	__DEFINE_SPECIAL_MODE_PARAMS(FIBRE),
+	__DEFINE_SPECIAL_MODE_PARAMS(BNC),
+	__DEFINE_LINK_MODE_PARAMS(10000, T, Full),
+	__DEFINE_SPECIAL_MODE_PARAMS(Pause),
+	__DEFINE_SPECIAL_MODE_PARAMS(Asym_Pause),
+	__DEFINE_LINK_MODE_PARAMS(2500, X, Full),
+	__DEFINE_SPECIAL_MODE_PARAMS(Backplane),
+	__DEFINE_LINK_MODE_PARAMS(1000, KX, Full),
+	__DEFINE_LINK_MODE_PARAMS(10000, KX4, Full),
+	__DEFINE_LINK_MODE_PARAMS(10000, KR, Full),
+	[ETHTOOL_LINK_MODE_10000baseR_FEC_BIT] = {
+		.speed	= SPEED_10000,
+		.duplex = DUPLEX_FULL,
+	},
+	__DEFINE_LINK_MODE_PARAMS(20000, MLD2, Full),
+	__DEFINE_LINK_MODE_PARAMS(20000, KR2, Full),
+	__DEFINE_LINK_MODE_PARAMS(40000, KR4, Full),
+	__DEFINE_LINK_MODE_PARAMS(40000, CR4, Full),
+	__DEFINE_LINK_MODE_PARAMS(40000, SR4, Full),
+	__DEFINE_LINK_MODE_PARAMS(40000, LR4, Full),
+	__DEFINE_LINK_MODE_PARAMS(56000, KR4, Full),
+	__DEFINE_LINK_MODE_PARAMS(56000, CR4, Full),
+	__DEFINE_LINK_MODE_PARAMS(56000, SR4, Full),
+	__DEFINE_LINK_MODE_PARAMS(56000, LR4, Full),
+	__DEFINE_LINK_MODE_PARAMS(25000, CR, Full),
+	__DEFINE_LINK_MODE_PARAMS(25000, KR, Full),
+	__DEFINE_LINK_MODE_PARAMS(25000, SR, Full),
+	__DEFINE_LINK_MODE_PARAMS(50000, CR2, Full),
+	__DEFINE_LINK_MODE_PARAMS(50000, KR2, Full),
+	__DEFINE_LINK_MODE_PARAMS(100000, KR4, Full),
+	__DEFINE_LINK_MODE_PARAMS(100000, SR4, Full),
+	__DEFINE_LINK_MODE_PARAMS(100000, CR4, Full),
+	__DEFINE_LINK_MODE_PARAMS(100000, LR4_ER4, Full),
+	__DEFINE_LINK_MODE_PARAMS(50000, SR2, Full),
+	__DEFINE_LINK_MODE_PARAMS(1000, X, Full),
+	__DEFINE_LINK_MODE_PARAMS(10000, CR, Full),
+	__DEFINE_LINK_MODE_PARAMS(10000, SR, Full),
+	__DEFINE_LINK_MODE_PARAMS(10000, LR, Full),
+	__DEFINE_LINK_MODE_PARAMS(10000, LRM, Full),
+	__DEFINE_LINK_MODE_PARAMS(10000, ER, Full),
+	__DEFINE_LINK_MODE_PARAMS(2500, T, Full),
+	__DEFINE_LINK_MODE_PARAMS(5000, T, Full),
+	__DEFINE_SPECIAL_MODE_PARAMS(FEC_NONE),
+	__DEFINE_SPECIAL_MODE_PARAMS(FEC_RS),
+	__DEFINE_SPECIAL_MODE_PARAMS(FEC_BASER),
+	__DEFINE_LINK_MODE_PARAMS(50000, KR, Full),
+	__DEFINE_LINK_MODE_PARAMS(50000, SR, Full),
+	__DEFINE_LINK_MODE_PARAMS(50000, CR, Full),
+	__DEFINE_LINK_MODE_PARAMS(50000, LR_ER_FR, Full),
+	__DEFINE_LINK_MODE_PARAMS(50000, DR, Full),
+	__DEFINE_LINK_MODE_PARAMS(100000, KR2, Full),
+	__DEFINE_LINK_MODE_PARAMS(100000, SR2, Full),
+	__DEFINE_LINK_MODE_PARAMS(100000, CR2, Full),
+	__DEFINE_LINK_MODE_PARAMS(100000, LR2_ER2_FR2, Full),
+	__DEFINE_LINK_MODE_PARAMS(100000, DR2, Full),
+	__DEFINE_LINK_MODE_PARAMS(200000, KR4, Full),
+	__DEFINE_LINK_MODE_PARAMS(200000, SR4, Full),
+	__DEFINE_LINK_MODE_PARAMS(200000, LR4_ER4_FR4, Full),
+	__DEFINE_LINK_MODE_PARAMS(200000, DR4, Full),
+	__DEFINE_LINK_MODE_PARAMS(200000, CR4, Full),
+	__DEFINE_LINK_MODE_PARAMS(100, T1, Full),
+	__DEFINE_LINK_MODE_PARAMS(1000, T1, Full),
+	__DEFINE_LINK_MODE_PARAMS(400000, KR8, Full),
+	__DEFINE_LINK_MODE_PARAMS(400000, SR8, Full),
+	__DEFINE_LINK_MODE_PARAMS(400000, LR8_ER8_FR8, Full),
+	__DEFINE_LINK_MODE_PARAMS(400000, DR8, Full),
+	__DEFINE_LINK_MODE_PARAMS(400000, CR8, Full),
+	__DEFINE_SPECIAL_MODE_PARAMS(FEC_LLRS),
+	__DEFINE_LINK_MODE_PARAMS(100000, KR, Full),
+	__DEFINE_LINK_MODE_PARAMS(100000, SR, Full),
+	__DEFINE_LINK_MODE_PARAMS(100000, LR_ER_FR, Full),
+	__DEFINE_LINK_MODE_PARAMS(100000, DR, Full),
+	__DEFINE_LINK_MODE_PARAMS(100000, CR, Full),
+	__DEFINE_LINK_MODE_PARAMS(200000, KR2, Full),
+	__DEFINE_LINK_MODE_PARAMS(200000, SR2, Full),
+	__DEFINE_LINK_MODE_PARAMS(200000, LR2_ER2_FR2, Full),
+	__DEFINE_LINK_MODE_PARAMS(200000, DR2, Full),
+	__DEFINE_LINK_MODE_PARAMS(200000, CR2, Full),
+	__DEFINE_LINK_MODE_PARAMS(400000, KR4, Full),
+	__DEFINE_LINK_MODE_PARAMS(400000, SR4, Full),
+	__DEFINE_LINK_MODE_PARAMS(400000, LR4_ER4_FR4, Full),
+	__DEFINE_LINK_MODE_PARAMS(400000, DR4, Full),
+	__DEFINE_LINK_MODE_PARAMS(400000, CR4, Full),
+	__DEFINE_LINK_MODE_PARAMS(100, FX, Half),
+	__DEFINE_LINK_MODE_PARAMS(100, FX, Full),
+};
+static_assert(ARRAY_SIZE(link_mode_params) == __ETHTOOL_LINK_MODE_MASK_NBITS);
+
 const char netif_msg_class_names[][ETH_GSTRING_LEN] = {
 	[NETIF_MSG_DRV_BIT]		= "drv",
 	[NETIF_MSG_PROBE_BIT]		= "probe",
diff --git a/net/ethtool/common.h b/net/ethtool/common.h
index 3d9251c95a8b..a9d071248698 100644
--- a/net/ethtool/common.h
+++ b/net/ethtool/common.h
@@ -14,6 +14,12 @@
 
 #define __SOF_TIMESTAMPING_CNT (const_ilog2(SOF_TIMESTAMPING_LAST) + 1)
 
+struct link_mode_info {
+	int				speed;
+	u8				lanes;
+	u8				duplex;
+};
+
 extern const char
 netdev_features_strings[NETDEV_FEATURE_COUNT][ETH_GSTRING_LEN];
 extern const char
@@ -23,6 +29,7 @@ tunable_strings[__ETHTOOL_TUNABLE_COUNT][ETH_GSTRING_LEN];
 extern const char
 phy_tunable_strings[__ETHTOOL_PHY_TUNABLE_COUNT][ETH_GSTRING_LEN];
 extern const char link_mode_names[][ETH_GSTRING_LEN];
+extern const struct link_mode_info link_mode_params[];
 extern const char netif_msg_class_names[][ETH_GSTRING_LEN];
 extern const char wol_mode_names[][ETH_GSTRING_LEN];
 extern const char sof_timestamping_names[][ETH_GSTRING_LEN];
diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
index 771688e1b0da..24783b71c584 100644
--- a/net/ethtool/ioctl.c
+++ b/net/ethtool/ioctl.c
@@ -426,13 +426,29 @@ struct ethtool_link_usettings {
 int __ethtool_get_link_ksettings(struct net_device *dev,
 				 struct ethtool_link_ksettings *link_ksettings)
 {
+	const struct link_mode_info *link_info;
+	int err;
+
 	ASSERT_RTNL();
 
 	if (!dev->ethtool_ops->get_link_ksettings)
 		return -EOPNOTSUPP;
 
 	memset(link_ksettings, 0, sizeof(*link_ksettings));
-	return dev->ethtool_ops->get_link_ksettings(dev, link_ksettings);
+
+	link_ksettings->link_mode = -1;
+	err = dev->ethtool_ops->get_link_ksettings(dev, link_ksettings);
+	if (err)
+		return err;
+
+	if (link_ksettings->link_mode != -1) {
+		link_info = &link_mode_params[link_ksettings->link_mode];
+		link_ksettings->base.speed = link_info->speed;
+		link_ksettings->lanes = link_info->lanes;
+		link_ksettings->base.duplex = link_info->duplex;
+	}
+
+	return 0;
 }
 EXPORT_SYMBOL(__ethtool_get_link_ksettings);
 
diff --git a/net/ethtool/linkmodes.c b/net/ethtool/linkmodes.c
index db3e31fc6709..fc986d035b01 100644
--- a/net/ethtool/linkmodes.c
+++ b/net/ethtool/linkmodes.c
@@ -4,6 +4,8 @@
 #include "common.h"
 #include "bitset.h"
 
+/* LINKMODES_GET */
+
 struct linkmodes_req_info {
 	struct ethnl_req_info		base;
 };
@@ -150,158 +152,6 @@ const struct ethnl_request_ops ethnl_linkmodes_request_ops = {
 
 /* LINKMODES_SET */
 
-struct link_mode_info {
-	int				speed;
-	u8				lanes;
-	u8				duplex;
-};
-
-#define __LINK_MODE_LANES_CR		1
-#define __LINK_MODE_LANES_CR2		2
-#define __LINK_MODE_LANES_CR4		4
-#define __LINK_MODE_LANES_CR8		8
-#define __LINK_MODE_LANES_DR		1
-#define __LINK_MODE_LANES_DR2		2
-#define __LINK_MODE_LANES_DR4		4
-#define __LINK_MODE_LANES_DR8		8
-#define __LINK_MODE_LANES_KR		1
-#define __LINK_MODE_LANES_KR2		2
-#define __LINK_MODE_LANES_KR4		4
-#define __LINK_MODE_LANES_KR8		8
-#define __LINK_MODE_LANES_SR		1
-#define __LINK_MODE_LANES_SR2		2
-#define __LINK_MODE_LANES_SR4		4
-#define __LINK_MODE_LANES_SR8		8
-#define __LINK_MODE_LANES_ER		1
-#define __LINK_MODE_LANES_KX		1
-#define __LINK_MODE_LANES_KX4		4
-#define __LINK_MODE_LANES_LR		1
-#define __LINK_MODE_LANES_LR4		4
-#define __LINK_MODE_LANES_LR4_ER4	4
-#define __LINK_MODE_LANES_LR_ER_FR	1
-#define __LINK_MODE_LANES_LR2_ER2_FR2	2
-#define __LINK_MODE_LANES_LR4_ER4_FR4	4
-#define __LINK_MODE_LANES_LR8_ER8_FR8	8
-#define __LINK_MODE_LANES_LRM		1
-#define __LINK_MODE_LANES_MLD2		2
-#define __LINK_MODE_LANES_T		1
-#define __LINK_MODE_LANES_T1		1
-#define __LINK_MODE_LANES_X		1
-#define __LINK_MODE_LANES_FX		1
-
-#define __DEFINE_LINK_MODE_PARAMS(_speed, _type, _duplex)	\
-	[ETHTOOL_LINK_MODE(_speed, _type, _duplex)] = {		\
-		.speed  = SPEED_ ## _speed, \
-		.lanes  = __LINK_MODE_LANES_ ## _type, \
-		.duplex	= __DUPLEX_ ## _duplex \
-	}
-#define __DUPLEX_Half DUPLEX_HALF
-#define __DUPLEX_Full DUPLEX_FULL
-#define __DEFINE_SPECIAL_MODE_PARAMS(_mode) \
-	[ETHTOOL_LINK_MODE_ ## _mode ## _BIT] = { \
-		.speed	= SPEED_UNKNOWN, \
-		.lanes	= 0, \
-		.duplex	= DUPLEX_UNKNOWN, \
-	}
-
-static const struct link_mode_info link_mode_params[] = {
-	__DEFINE_LINK_MODE_PARAMS(10, T, Half),
-	__DEFINE_LINK_MODE_PARAMS(10, T, Full),
-	__DEFINE_LINK_MODE_PARAMS(100, T, Half),
-	__DEFINE_LINK_MODE_PARAMS(100, T, Full),
-	__DEFINE_LINK_MODE_PARAMS(1000, T, Half),
-	__DEFINE_LINK_MODE_PARAMS(1000, T, Full),
-	__DEFINE_SPECIAL_MODE_PARAMS(Autoneg),
-	__DEFINE_SPECIAL_MODE_PARAMS(TP),
-	__DEFINE_SPECIAL_MODE_PARAMS(AUI),
-	__DEFINE_SPECIAL_MODE_PARAMS(MII),
-	__DEFINE_SPECIAL_MODE_PARAMS(FIBRE),
-	__DEFINE_SPECIAL_MODE_PARAMS(BNC),
-	__DEFINE_LINK_MODE_PARAMS(10000, T, Full),
-	__DEFINE_SPECIAL_MODE_PARAMS(Pause),
-	__DEFINE_SPECIAL_MODE_PARAMS(Asym_Pause),
-	__DEFINE_LINK_MODE_PARAMS(2500, X, Full),
-	__DEFINE_SPECIAL_MODE_PARAMS(Backplane),
-	__DEFINE_LINK_MODE_PARAMS(1000, KX, Full),
-	__DEFINE_LINK_MODE_PARAMS(10000, KX4, Full),
-	__DEFINE_LINK_MODE_PARAMS(10000, KR, Full),
-	[ETHTOOL_LINK_MODE_10000baseR_FEC_BIT] = {
-		.speed	= SPEED_10000,
-		.duplex = DUPLEX_FULL,
-	},
-	__DEFINE_LINK_MODE_PARAMS(20000, MLD2, Full),
-	__DEFINE_LINK_MODE_PARAMS(20000, KR2, Full),
-	__DEFINE_LINK_MODE_PARAMS(40000, KR4, Full),
-	__DEFINE_LINK_MODE_PARAMS(40000, CR4, Full),
-	__DEFINE_LINK_MODE_PARAMS(40000, SR4, Full),
-	__DEFINE_LINK_MODE_PARAMS(40000, LR4, Full),
-	__DEFINE_LINK_MODE_PARAMS(56000, KR4, Full),
-	__DEFINE_LINK_MODE_PARAMS(56000, CR4, Full),
-	__DEFINE_LINK_MODE_PARAMS(56000, SR4, Full),
-	__DEFINE_LINK_MODE_PARAMS(56000, LR4, Full),
-	__DEFINE_LINK_MODE_PARAMS(25000, CR, Full),
-	__DEFINE_LINK_MODE_PARAMS(25000, KR, Full),
-	__DEFINE_LINK_MODE_PARAMS(25000, SR, Full),
-	__DEFINE_LINK_MODE_PARAMS(50000, CR2, Full),
-	__DEFINE_LINK_MODE_PARAMS(50000, KR2, Full),
-	__DEFINE_LINK_MODE_PARAMS(100000, KR4, Full),
-	__DEFINE_LINK_MODE_PARAMS(100000, SR4, Full),
-	__DEFINE_LINK_MODE_PARAMS(100000, CR4, Full),
-	__DEFINE_LINK_MODE_PARAMS(100000, LR4_ER4, Full),
-	__DEFINE_LINK_MODE_PARAMS(50000, SR2, Full),
-	__DEFINE_LINK_MODE_PARAMS(1000, X, Full),
-	__DEFINE_LINK_MODE_PARAMS(10000, CR, Full),
-	__DEFINE_LINK_MODE_PARAMS(10000, SR, Full),
-	__DEFINE_LINK_MODE_PARAMS(10000, LR, Full),
-	__DEFINE_LINK_MODE_PARAMS(10000, LRM, Full),
-	__DEFINE_LINK_MODE_PARAMS(10000, ER, Full),
-	__DEFINE_LINK_MODE_PARAMS(2500, T, Full),
-	__DEFINE_LINK_MODE_PARAMS(5000, T, Full),
-	__DEFINE_SPECIAL_MODE_PARAMS(FEC_NONE),
-	__DEFINE_SPECIAL_MODE_PARAMS(FEC_RS),
-	__DEFINE_SPECIAL_MODE_PARAMS(FEC_BASER),
-	__DEFINE_LINK_MODE_PARAMS(50000, KR, Full),
-	__DEFINE_LINK_MODE_PARAMS(50000, SR, Full),
-	__DEFINE_LINK_MODE_PARAMS(50000, CR, Full),
-	__DEFINE_LINK_MODE_PARAMS(50000, LR_ER_FR, Full),
-	__DEFINE_LINK_MODE_PARAMS(50000, DR, Full),
-	__DEFINE_LINK_MODE_PARAMS(100000, KR2, Full),
-	__DEFINE_LINK_MODE_PARAMS(100000, SR2, Full),
-	__DEFINE_LINK_MODE_PARAMS(100000, CR2, Full),
-	__DEFINE_LINK_MODE_PARAMS(100000, LR2_ER2_FR2, Full),
-	__DEFINE_LINK_MODE_PARAMS(100000, DR2, Full),
-	__DEFINE_LINK_MODE_PARAMS(200000, KR4, Full),
-	__DEFINE_LINK_MODE_PARAMS(200000, SR4, Full),
-	__DEFINE_LINK_MODE_PARAMS(200000, LR4_ER4_FR4, Full),
-	__DEFINE_LINK_MODE_PARAMS(200000, DR4, Full),
-	__DEFINE_LINK_MODE_PARAMS(200000, CR4, Full),
-	__DEFINE_LINK_MODE_PARAMS(100, T1, Full),
-	__DEFINE_LINK_MODE_PARAMS(1000, T1, Full),
-	__DEFINE_LINK_MODE_PARAMS(400000, KR8, Full),
-	__DEFINE_LINK_MODE_PARAMS(400000, SR8, Full),
-	__DEFINE_LINK_MODE_PARAMS(400000, LR8_ER8_FR8, Full),
-	__DEFINE_LINK_MODE_PARAMS(400000, DR8, Full),
-	__DEFINE_LINK_MODE_PARAMS(400000, CR8, Full),
-	__DEFINE_SPECIAL_MODE_PARAMS(FEC_LLRS),
-	__DEFINE_LINK_MODE_PARAMS(100000, KR, Full),
-	__DEFINE_LINK_MODE_PARAMS(100000, SR, Full),
-	__DEFINE_LINK_MODE_PARAMS(100000, LR_ER_FR, Full),
-	__DEFINE_LINK_MODE_PARAMS(100000, DR, Full),
-	__DEFINE_LINK_MODE_PARAMS(100000, CR, Full),
-	__DEFINE_LINK_MODE_PARAMS(200000, KR2, Full),
-	__DEFINE_LINK_MODE_PARAMS(200000, SR2, Full),
-	__DEFINE_LINK_MODE_PARAMS(200000, LR2_ER2_FR2, Full),
-	__DEFINE_LINK_MODE_PARAMS(200000, DR2, Full),
-	__DEFINE_LINK_MODE_PARAMS(200000, CR2, Full),
-	__DEFINE_LINK_MODE_PARAMS(400000, KR4, Full),
-	__DEFINE_LINK_MODE_PARAMS(400000, SR4, Full),
-	__DEFINE_LINK_MODE_PARAMS(400000, LR4_ER4_FR4, Full),
-	__DEFINE_LINK_MODE_PARAMS(400000, DR4, Full),
-	__DEFINE_LINK_MODE_PARAMS(400000, CR4, Full),
-	__DEFINE_LINK_MODE_PARAMS(100, FX, Half),
-	__DEFINE_LINK_MODE_PARAMS(100, FX, Full),
-};
-
 const struct nla_policy ethnl_linkmodes_set_policy[] = {
 	[ETHTOOL_A_LINKMODES_HEADER]		=
 		NLA_POLICY_NESTED(ethnl_header_policy),
@@ -327,9 +177,6 @@ static bool ethnl_auto_linkmodes(struct ethtool_link_ksettings *ksettings,
 	DECLARE_BITMAP(old_adv, __ETHTOOL_LINK_MODE_MASK_NBITS);
 	unsigned int i;
 
-	BUILD_BUG_ON(ARRAY_SIZE(link_mode_params) !=
-		     __ETHTOOL_LINK_MODE_MASK_NBITS);
-
 	bitmap_copy(old_adv, advertising, __ETHTOOL_LINK_MODE_MASK_NBITS);
 
 	for (i = 0; i < __ETHTOOL_LINK_MODE_MASK_NBITS; i++) {
-- 
2.26.2


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

* [PATCH net-next v4 4/8] ethtool: Expose the number of lanes in use
  2021-02-02 18:06 [PATCH net-next v4 0/8] Support setting lanes via ethtool Danielle Ratson
                   ` (2 preceding siblings ...)
  2021-02-02 18:06 ` [PATCH net-next v4 3/8] ethtool: Get link mode in use instead of speed and duplex parameters Danielle Ratson
@ 2021-02-02 18:06 ` Danielle Ratson
  2021-02-02 18:06 ` [PATCH net-next v4 5/8] mlxsw: ethtool: Remove max lanes filtering Danielle Ratson
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Danielle Ratson @ 2021-02-02 18:06 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, jiri, andrew, f.fainelli, mkubecek, mlxsw, idosch,
	Danielle Ratson

Currently, ethtool does not expose how many lanes are used when the
link is up.

After adding a possibility to advertise or force a specific number of
lanes, the lanes in use value can be either the maximum width of the port
or below.

Extend ethtool to expose the number of lanes currently in use for
drivers that support it.

For example:

$ ethtool -s swp1 speed 100000 lanes 4
$ ethtool -s swp2 speed 100000 lanes 4
$ ip link set swp1 up
$ ip link set swp2 up
$ ethtool swp1
Settings for swp1:
        Supported ports: [ FIBRE         Backplane ]
        Supported link modes:   1000baseT/Full
                                10000baseT/Full
                                1000baseKX/Full
                                10000baseKR/Full
                                10000baseR_FEC
                                40000baseKR4/Full
                                40000baseCR4/Full
                                40000baseSR4/Full
                                40000baseLR4/Full
                                25000baseCR/Full
                                25000baseKR/Full
                                25000baseSR/Full
                                50000baseCR2/Full
                                50000baseKR2/Full
                                100000baseKR4/Full
                                100000baseSR4/Full
                                100000baseCR4/Full
                                100000baseLR4_ER4/Full
                                50000baseSR2/Full
                                10000baseCR/Full
                                10000baseSR/Full
                                10000baseLR/Full
                                10000baseER/Full
                                50000baseKR/Full
                                50000baseSR/Full
                                50000baseCR/Full
                                50000baseLR_ER_FR/Full
                                50000baseDR/Full
                                100000baseKR2/Full
                                100000baseSR2/Full
                                100000baseCR2/Full
                                100000baseLR2_ER2_FR2/Full
                                100000baseDR2/Full
                                200000baseKR4/Full
                                200000baseSR4/Full
                                200000baseLR4_ER4_FR4/Full
                                200000baseDR4/Full
                                200000baseCR4/Full
        Supported pause frame use: Symmetric Receive-only
        Supports auto-negotiation: Yes
        Supported FEC modes: Not reported
        Advertised link modes:  1000baseT/Full
                                10000baseT/Full
                                1000baseKX/Full
                                1000baseKX/Full
                                10000baseKR/Full
                                10000baseR_FEC
                                40000baseKR4/Full
                                40000baseCR4/Full
                                40000baseSR4/Full
                                40000baseLR4/Full
                                25000baseCR/Full
                                25000baseKR/Full
                                25000baseSR/Full
                                50000baseCR2/Full
                                50000baseKR2/Full
                                100000baseKR4/Full
                                100000baseSR4/Full
                                100000baseCR4/Full
                                100000baseLR4_ER4/Full
                                50000baseSR2/Full
                                10000baseCR/Full
                                10000baseSR/Full
                                10000baseLR/Full
                                10000baseER/Full
                                200000baseKR4/Full
                                200000baseSR4/Full
                                200000baseLR4_ER4_FR4/Full
                                200000baseDR4/Full
                                200000baseCR4/Full
        Advertised pause frame use: No
        Advertised auto-negotiation: Yes
        Advertised FEC modes: Not reported
        Advertised link modes:  100000baseKR4/Full
                                100000baseSR4/Full
                                100000baseCR4/Full
                                100000baseLR4_ER4/Full
	Advertised pause frame use: No
	Advertised auto-negotiation: Yes
	Advertised FEC modes: Not reported
	Speed: 100000Mb/s
	Lanes: 4
	Duplex: Full
	Auto-negotiation: on
	Port: Direct Attach Copper
	PHYAD: 0
	Transceiver: internal
	Link detected: yes

Signed-off-by: Danielle Ratson <danieller@nvidia.com>
---

Notes:
    v3:
    	* Change the condition for supporting the lanes parameter.
    
    v2:
    	* Reword commit message.
    	* Since now we get a link mode from driver, instead of each
    	  parameter separately, simply derive lanes from it.

 net/ethtool/linkmodes.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/net/ethtool/linkmodes.c b/net/ethtool/linkmodes.c
index fc986d035b01..f9eda596f301 100644
--- a/net/ethtool/linkmodes.c
+++ b/net/ethtool/linkmodes.c
@@ -45,6 +45,9 @@ static int linkmodes_prepare_data(const struct ethnl_req_info *req_base,
 		goto out;
 	}
 
+	if (!dev->ethtool_ops->cap_link_lanes_supported)
+		data->ksettings.lanes = 0;
+
 	data->peer_empty =
 		bitmap_empty(data->ksettings.link_modes.lp_advertising,
 			     __ETHTOOL_LINK_MODE_MASK_NBITS);
@@ -65,6 +68,7 @@ static int linkmodes_reply_size(const struct ethnl_req_info *req_base,
 
 	len = nla_total_size(sizeof(u8)) /* LINKMODES_AUTONEG */
 		+ nla_total_size(sizeof(u32)) /* LINKMODES_SPEED */
+		+ nla_total_size(sizeof(u32)) /* LINKMODES_LANES */
 		+ nla_total_size(sizeof(u8)) /* LINKMODES_DUPLEX */
 		+ 0;
 	ret = ethnl_bitset_size(ksettings->link_modes.advertising,
@@ -125,6 +129,10 @@ static int linkmodes_fill_reply(struct sk_buff *skb,
 	    nla_put_u8(skb, ETHTOOL_A_LINKMODES_DUPLEX, lsettings->duplex))
 		return -EMSGSIZE;
 
+	if (ksettings->lanes &&
+	    nla_put_u32(skb, ETHTOOL_A_LINKMODES_LANES, ksettings->lanes))
+		return -EMSGSIZE;
+
 	if (lsettings->master_slave_cfg != MASTER_SLAVE_CFG_UNSUPPORTED &&
 	    nla_put_u8(skb, ETHTOOL_A_LINKMODES_MASTER_SLAVE_CFG,
 		       lsettings->master_slave_cfg))
-- 
2.26.2


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

* [PATCH net-next v4 5/8] mlxsw: ethtool: Remove max lanes filtering
  2021-02-02 18:06 [PATCH net-next v4 0/8] Support setting lanes via ethtool Danielle Ratson
                   ` (3 preceding siblings ...)
  2021-02-02 18:06 ` [PATCH net-next v4 4/8] ethtool: Expose the number of lanes in use Danielle Ratson
@ 2021-02-02 18:06 ` Danielle Ratson
  2021-02-02 18:06 ` [PATCH net-next v4 6/8] mlxsw: ethtool: Add support for setting lanes when autoneg is off Danielle Ratson
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Danielle Ratson @ 2021-02-02 18:06 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, jiri, andrew, f.fainelli, mkubecek, mlxsw, idosch,
	Danielle Ratson

Currently, when a speed can be supported by different number of lanes,
the supported link modes bitmask contains only link modes with a single
number of lanes.

This was done in order to prevent auto negotiation on number of
lanes after 50G-1-lane and 100G-2-lanes link modes were introduced.

For example, if a port's max width is 4, only link modes with 4 lanes
will be presented as supported by that port, so 100G is always achieved by
4 lanes of 25G.

After the previous patches that allow selection of the number of lanes,
auto negotiation on number of lanes becomes practical.

Remove that filtering of the maximum number of lanes supported link modes,
so indeed all the supported and advertised link modes will be shown.

Signed-off-by: Danielle Ratson <danieller@nvidia.com>
Reviewed-by: Jiri Pirko <jiri@nvidia.com>
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---
 .../net/ethernet/mellanox/mlxsw/spectrum.h    |  4 +--
 .../mellanox/mlxsw/spectrum_ethtool.c         | 33 ++++++++-----------
 2 files changed, 15 insertions(+), 22 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
index b1b593076a76..cc4aeb3cdd10 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
@@ -329,13 +329,13 @@ struct mlxsw_sp_port_type_speed_ops {
 					 u32 ptys_eth_proto,
 					 struct ethtool_link_ksettings *cmd);
 	void (*from_ptys_link)(struct mlxsw_sp *mlxsw_sp, u32 ptys_eth_proto,
-			       u8 width, unsigned long *mode);
+			       unsigned long *mode);
 	u32 (*from_ptys_speed)(struct mlxsw_sp *mlxsw_sp, u32 ptys_eth_proto);
 	void (*from_ptys_speed_duplex)(struct mlxsw_sp *mlxsw_sp,
 				       bool carrier_ok, u32 ptys_eth_proto,
 				       struct ethtool_link_ksettings *cmd);
 	int (*ptys_max_speed)(struct mlxsw_sp_port *mlxsw_sp_port, u32 *p_max_speed);
-	u32 (*to_ptys_advert_link)(struct mlxsw_sp *mlxsw_sp, u8 width,
+	u32 (*to_ptys_advert_link)(struct mlxsw_sp *mlxsw_sp,
 				   const struct ethtool_link_ksettings *cmd);
 	u32 (*to_ptys_speed)(struct mlxsw_sp *mlxsw_sp, u8 width, u32 speed);
 	void (*reg_ptys_eth_pack)(struct mlxsw_sp *mlxsw_sp, char *payload,
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_ethtool.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_ethtool.c
index 41288144852d..aa13af0f33f0 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_ethtool.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_ethtool.c
@@ -858,7 +858,7 @@ static int mlxsw_sp_port_get_sset_count(struct net_device *dev, int sset)
 
 static void
 mlxsw_sp_port_get_link_supported(struct mlxsw_sp *mlxsw_sp, u32 eth_proto_cap,
-				 u8 width, struct ethtool_link_ksettings *cmd)
+				 struct ethtool_link_ksettings *cmd)
 {
 	const struct mlxsw_sp_port_type_speed_ops *ops;
 
@@ -869,13 +869,13 @@ mlxsw_sp_port_get_link_supported(struct mlxsw_sp *mlxsw_sp, u32 eth_proto_cap,
 	ethtool_link_ksettings_add_link_mode(cmd, supported, Pause);
 
 	ops->from_ptys_supported_port(mlxsw_sp, eth_proto_cap, cmd);
-	ops->from_ptys_link(mlxsw_sp, eth_proto_cap, width,
+	ops->from_ptys_link(mlxsw_sp, eth_proto_cap,
 			    cmd->link_modes.supported);
 }
 
 static void
 mlxsw_sp_port_get_link_advertise(struct mlxsw_sp *mlxsw_sp,
-				 u32 eth_proto_admin, bool autoneg, u8 width,
+				 u32 eth_proto_admin, bool autoneg,
 				 struct ethtool_link_ksettings *cmd)
 {
 	const struct mlxsw_sp_port_type_speed_ops *ops;
@@ -886,7 +886,7 @@ mlxsw_sp_port_get_link_advertise(struct mlxsw_sp *mlxsw_sp,
 		return;
 
 	ethtool_link_ksettings_add_link_mode(cmd, advertising, Autoneg);
-	ops->from_ptys_link(mlxsw_sp, eth_proto_admin, width,
+	ops->from_ptys_link(mlxsw_sp, eth_proto_admin,
 			    cmd->link_modes.advertising);
 }
 
@@ -960,11 +960,9 @@ static int mlxsw_sp_port_get_link_ksettings(struct net_device *dev,
 	ops = mlxsw_sp->port_type_speed_ops;
 	autoneg = mlxsw_sp_port->link.autoneg;
 
-	mlxsw_sp_port_get_link_supported(mlxsw_sp, eth_proto_cap,
-					 mlxsw_sp_port->mapping.width, cmd);
+	mlxsw_sp_port_get_link_supported(mlxsw_sp, eth_proto_cap, cmd);
 
-	mlxsw_sp_port_get_link_advertise(mlxsw_sp, eth_proto_admin, autoneg,
-					 mlxsw_sp_port->mapping.width, cmd);
+	mlxsw_sp_port_get_link_advertise(mlxsw_sp, eth_proto_admin, autoneg, cmd);
 
 	cmd->base.autoneg = autoneg ? AUTONEG_ENABLE : AUTONEG_DISABLE;
 	cmd->base.port = mlxsw_sp_port_connector_port(connector_type);
@@ -997,8 +995,7 @@ mlxsw_sp_port_set_link_ksettings(struct net_device *dev,
 
 	autoneg = cmd->base.autoneg == AUTONEG_ENABLE;
 	eth_proto_new = autoneg ?
-		ops->to_ptys_advert_link(mlxsw_sp, mlxsw_sp_port->mapping.width,
-					 cmd) :
+		ops->to_ptys_advert_link(mlxsw_sp, cmd) :
 		ops->to_ptys_speed(mlxsw_sp, mlxsw_sp_port->mapping.width,
 				   cmd->base.speed);
 
@@ -1200,7 +1197,7 @@ mlxsw_sp1_from_ptys_supported_port(struct mlxsw_sp *mlxsw_sp,
 
 static void
 mlxsw_sp1_from_ptys_link(struct mlxsw_sp *mlxsw_sp, u32 ptys_eth_proto,
-			 u8 width, unsigned long *mode)
+			 unsigned long *mode)
 {
 	int i;
 
@@ -1262,7 +1259,7 @@ static int mlxsw_sp1_ptys_max_speed(struct mlxsw_sp_port *mlxsw_sp_port, u32 *p_
 }
 
 static u32
-mlxsw_sp1_to_ptys_advert_link(struct mlxsw_sp *mlxsw_sp, u8 width,
+mlxsw_sp1_to_ptys_advert_link(struct mlxsw_sp *mlxsw_sp,
 			      const struct ethtool_link_ksettings *cmd)
 {
 	u32 ptys_proto = 0;
@@ -1621,14 +1618,12 @@ mlxsw_sp2_set_bit_ethtool(const struct mlxsw_sp2_port_link_mode *link_mode,
 
 static void
 mlxsw_sp2_from_ptys_link(struct mlxsw_sp *mlxsw_sp, u32 ptys_eth_proto,
-			 u8 width, unsigned long *mode)
+			 unsigned long *mode)
 {
-	u8 mask_width = mlxsw_sp_port_mask_width_get(width);
 	int i;
 
 	for (i = 0; i < MLXSW_SP2_PORT_LINK_MODE_LEN; i++) {
-		if ((ptys_eth_proto & mlxsw_sp2_port_link_mode[i].mask) &&
-		    (mask_width & mlxsw_sp2_port_link_mode[i].mask_width))
+		if (ptys_eth_proto & mlxsw_sp2_port_link_mode[i].mask)
 			mlxsw_sp2_set_bit_ethtool(&mlxsw_sp2_port_link_mode[i],
 						  mode);
 	}
@@ -1700,16 +1695,14 @@ mlxsw_sp2_test_bit_ethtool(const struct mlxsw_sp2_port_link_mode *link_mode,
 }
 
 static u32
-mlxsw_sp2_to_ptys_advert_link(struct mlxsw_sp *mlxsw_sp, u8 width,
+mlxsw_sp2_to_ptys_advert_link(struct mlxsw_sp *mlxsw_sp,
 			      const struct ethtool_link_ksettings *cmd)
 {
-	u8 mask_width = mlxsw_sp_port_mask_width_get(width);
 	u32 ptys_proto = 0;
 	int i;
 
 	for (i = 0; i < MLXSW_SP2_PORT_LINK_MODE_LEN; i++) {
-		if ((mask_width & mlxsw_sp2_port_link_mode[i].mask_width) &&
-		    mlxsw_sp2_test_bit_ethtool(&mlxsw_sp2_port_link_mode[i],
+		if (mlxsw_sp2_test_bit_ethtool(&mlxsw_sp2_port_link_mode[i],
 					       cmd->link_modes.advertising))
 			ptys_proto |= mlxsw_sp2_port_link_mode[i].mask;
 	}
-- 
2.26.2


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

* [PATCH net-next v4 6/8] mlxsw: ethtool: Add support for setting lanes when autoneg is off
  2021-02-02 18:06 [PATCH net-next v4 0/8] Support setting lanes via ethtool Danielle Ratson
                   ` (4 preceding siblings ...)
  2021-02-02 18:06 ` [PATCH net-next v4 5/8] mlxsw: ethtool: Remove max lanes filtering Danielle Ratson
@ 2021-02-02 18:06 ` Danielle Ratson
  2021-02-02 18:06 ` [PATCH net-next v4 7/8] mlxsw: ethtool: Pass link mode in use to ethtool Danielle Ratson
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Danielle Ratson @ 2021-02-02 18:06 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, jiri, andrew, f.fainelli, mkubecek, mlxsw, idosch,
	Danielle Ratson

Currently, when auto negotiation is set to off, the user can force a
specific speed or both speed and duplex. The user cannot influence the
number of lanes that will be forced.

Add support for setting speed along with lanes so one would be able
to choose how many lanes will be forced.

When lanes parameter is passed from user space, choose the link mode
that its actual width equals to it.
Otherwise, the default link mode will be the one that supports the width
of the port.

Signed-off-by: Danielle Ratson <danieller@nvidia.com>
---

Notes:
    v3:
    	* Re-set the bitfield of supporting lanes in the driver to 'true'.
    
    v2:
    	* Reword commit message.
    	* Add an actual width field for Spectrum-2 link modes, and change
    	  accordingly the conditions for choosing a link mode bit.

 .../net/ethernet/mellanox/mlxsw/spectrum.h    |   3 +-
 .../mellanox/mlxsw/spectrum_ethtool.c         | 116 ++++++++++++------
 2 files changed, 78 insertions(+), 41 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
index cc4aeb3cdd10..0ad6b8a581d5 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
@@ -337,7 +337,8 @@ struct mlxsw_sp_port_type_speed_ops {
 	int (*ptys_max_speed)(struct mlxsw_sp_port *mlxsw_sp_port, u32 *p_max_speed);
 	u32 (*to_ptys_advert_link)(struct mlxsw_sp *mlxsw_sp,
 				   const struct ethtool_link_ksettings *cmd);
-	u32 (*to_ptys_speed)(struct mlxsw_sp *mlxsw_sp, u8 width, u32 speed);
+	u32 (*to_ptys_speed_lanes)(struct mlxsw_sp *mlxsw_sp, u8 width,
+				   const struct ethtool_link_ksettings *cmd);
 	void (*reg_ptys_eth_pack)(struct mlxsw_sp *mlxsw_sp, char *payload,
 				  u8 local_port, u32 proto_admin, bool autoneg);
 	void (*reg_ptys_eth_unpack)(struct mlxsw_sp *mlxsw_sp, char *payload,
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_ethtool.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_ethtool.c
index aa13af0f33f0..b0031ae7e623 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_ethtool.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_ethtool.c
@@ -996,12 +996,12 @@ mlxsw_sp_port_set_link_ksettings(struct net_device *dev,
 	autoneg = cmd->base.autoneg == AUTONEG_ENABLE;
 	eth_proto_new = autoneg ?
 		ops->to_ptys_advert_link(mlxsw_sp, cmd) :
-		ops->to_ptys_speed(mlxsw_sp, mlxsw_sp_port->mapping.width,
-				   cmd->base.speed);
+		ops->to_ptys_speed_lanes(mlxsw_sp, mlxsw_sp_port->mapping.width,
+					 cmd);
 
 	eth_proto_new = eth_proto_new & eth_proto_cap;
 	if (!eth_proto_new) {
-		netdev_err(dev, "No supported speed requested\n");
+		netdev_err(dev, "No supported speed or lanes requested\n");
 		return -EINVAL;
 	}
 
@@ -1062,20 +1062,21 @@ mlxsw_sp_get_ts_info(struct net_device *netdev, struct ethtool_ts_info *info)
 }
 
 const struct ethtool_ops mlxsw_sp_port_ethtool_ops = {
-	.get_drvinfo		= mlxsw_sp_port_get_drvinfo,
-	.get_link		= ethtool_op_get_link,
-	.get_link_ext_state	= mlxsw_sp_port_get_link_ext_state,
-	.get_pauseparam		= mlxsw_sp_port_get_pauseparam,
-	.set_pauseparam		= mlxsw_sp_port_set_pauseparam,
-	.get_strings		= mlxsw_sp_port_get_strings,
-	.set_phys_id		= mlxsw_sp_port_set_phys_id,
-	.get_ethtool_stats	= mlxsw_sp_port_get_stats,
-	.get_sset_count		= mlxsw_sp_port_get_sset_count,
-	.get_link_ksettings	= mlxsw_sp_port_get_link_ksettings,
-	.set_link_ksettings	= mlxsw_sp_port_set_link_ksettings,
-	.get_module_info	= mlxsw_sp_get_module_info,
-	.get_module_eeprom	= mlxsw_sp_get_module_eeprom,
-	.get_ts_info		= mlxsw_sp_get_ts_info,
+	.cap_link_lanes_supported	= true,
+	.get_drvinfo			= mlxsw_sp_port_get_drvinfo,
+	.get_link			= ethtool_op_get_link,
+	.get_link_ext_state		= mlxsw_sp_port_get_link_ext_state,
+	.get_pauseparam			= mlxsw_sp_port_get_pauseparam,
+	.set_pauseparam			= mlxsw_sp_port_set_pauseparam,
+	.get_strings			= mlxsw_sp_port_get_strings,
+	.set_phys_id			= mlxsw_sp_port_set_phys_id,
+	.get_ethtool_stats		= mlxsw_sp_port_get_stats,
+	.get_sset_count			= mlxsw_sp_port_get_sset_count,
+	.get_link_ksettings		= mlxsw_sp_port_get_link_ksettings,
+	.set_link_ksettings		= mlxsw_sp_port_set_link_ksettings,
+	.get_module_info		= mlxsw_sp_get_module_info,
+	.get_module_eeprom		= mlxsw_sp_get_module_eeprom,
+	.get_ts_info			= mlxsw_sp_get_ts_info,
 };
 
 struct mlxsw_sp1_port_link_mode {
@@ -1273,14 +1274,17 @@ mlxsw_sp1_to_ptys_advert_link(struct mlxsw_sp *mlxsw_sp,
 	return ptys_proto;
 }
 
-static u32 mlxsw_sp1_to_ptys_speed(struct mlxsw_sp *mlxsw_sp, u8 width,
-				   u32 speed)
+static u32 mlxsw_sp1_to_ptys_speed_lanes(struct mlxsw_sp *mlxsw_sp, u8 width,
+					 const struct ethtool_link_ksettings *cmd)
 {
 	u32 ptys_proto = 0;
 	int i;
 
+	if (cmd->lanes > width)
+		return ptys_proto;
+
 	for (i = 0; i < MLXSW_SP1_PORT_LINK_MODE_LEN; i++) {
-		if (speed == mlxsw_sp1_port_link_mode[i].speed)
+		if (cmd->base.speed == mlxsw_sp1_port_link_mode[i].speed)
 			ptys_proto |= mlxsw_sp1_port_link_mode[i].mask;
 	}
 	return ptys_proto;
@@ -1323,7 +1327,7 @@ const struct mlxsw_sp_port_type_speed_ops mlxsw_sp1_port_type_speed_ops = {
 	.from_ptys_speed_duplex		= mlxsw_sp1_from_ptys_speed_duplex,
 	.ptys_max_speed			= mlxsw_sp1_ptys_max_speed,
 	.to_ptys_advert_link		= mlxsw_sp1_to_ptys_advert_link,
-	.to_ptys_speed			= mlxsw_sp1_to_ptys_speed,
+	.to_ptys_speed_lanes		= mlxsw_sp1_to_ptys_speed_lanes,
 	.reg_ptys_eth_pack		= mlxsw_sp1_reg_ptys_eth_pack,
 	.reg_ptys_eth_unpack		= mlxsw_sp1_reg_ptys_eth_unpack,
 	.ptys_proto_cap_masked_get	= mlxsw_sp1_ptys_proto_cap_masked_get,
@@ -1485,7 +1489,8 @@ struct mlxsw_sp2_port_link_mode {
 	int m_ethtool_len;
 	u32 mask;
 	u32 speed;
-	u8 mask_width;
+	u32 width;
+	u8 mask_sup_width;
 };
 
 static const struct mlxsw_sp2_port_link_mode mlxsw_sp2_port_link_mode[] = {
@@ -1493,105 +1498,117 @@ static const struct mlxsw_sp2_port_link_mode mlxsw_sp2_port_link_mode[] = {
 		.mask		= MLXSW_REG_PTYS_EXT_ETH_SPEED_SGMII_100M,
 		.mask_ethtool	= mlxsw_sp2_mask_ethtool_sgmii_100m,
 		.m_ethtool_len	= MLXSW_SP2_MASK_ETHTOOL_SGMII_100M_LEN,
-		.mask_width	= MLXSW_SP_PORT_MASK_WIDTH_1X |
+		.mask_sup_width	= MLXSW_SP_PORT_MASK_WIDTH_1X |
 				  MLXSW_SP_PORT_MASK_WIDTH_2X |
 				  MLXSW_SP_PORT_MASK_WIDTH_4X |
 				  MLXSW_SP_PORT_MASK_WIDTH_8X,
 		.speed		= SPEED_100,
+		.width		= 1,
 	},
 	{
 		.mask		= MLXSW_REG_PTYS_EXT_ETH_SPEED_1000BASE_X_SGMII,
 		.mask_ethtool	= mlxsw_sp2_mask_ethtool_1000base_x_sgmii,
 		.m_ethtool_len	= MLXSW_SP2_MASK_ETHTOOL_1000BASE_X_SGMII_LEN,
-		.mask_width	= MLXSW_SP_PORT_MASK_WIDTH_1X |
+		.mask_sup_width	= MLXSW_SP_PORT_MASK_WIDTH_1X |
 				  MLXSW_SP_PORT_MASK_WIDTH_2X |
 				  MLXSW_SP_PORT_MASK_WIDTH_4X |
 				  MLXSW_SP_PORT_MASK_WIDTH_8X,
 		.speed		= SPEED_1000,
+		.width		= 1,
 	},
 	{
 		.mask		= MLXSW_REG_PTYS_EXT_ETH_SPEED_5GBASE_R,
 		.mask_ethtool	= mlxsw_sp2_mask_ethtool_5gbase_r,
 		.m_ethtool_len	= MLXSW_SP2_MASK_ETHTOOL_5GBASE_R_LEN,
-		.mask_width	= MLXSW_SP_PORT_MASK_WIDTH_1X |
+		.mask_sup_width	= MLXSW_SP_PORT_MASK_WIDTH_1X |
 				  MLXSW_SP_PORT_MASK_WIDTH_2X |
 				  MLXSW_SP_PORT_MASK_WIDTH_4X |
 				  MLXSW_SP_PORT_MASK_WIDTH_8X,
 		.speed		= SPEED_5000,
+		.width		= 1,
 	},
 	{
 		.mask		= MLXSW_REG_PTYS_EXT_ETH_SPEED_XFI_XAUI_1_10G,
 		.mask_ethtool	= mlxsw_sp2_mask_ethtool_xfi_xaui_1_10g,
 		.m_ethtool_len	= MLXSW_SP2_MASK_ETHTOOL_XFI_XAUI_1_10G_LEN,
-		.mask_width	= MLXSW_SP_PORT_MASK_WIDTH_1X |
+		.mask_sup_width	= MLXSW_SP_PORT_MASK_WIDTH_1X |
 				  MLXSW_SP_PORT_MASK_WIDTH_2X |
 				  MLXSW_SP_PORT_MASK_WIDTH_4X |
 				  MLXSW_SP_PORT_MASK_WIDTH_8X,
 		.speed		= SPEED_10000,
+		.width		= 1,
 	},
 	{
 		.mask		= MLXSW_REG_PTYS_EXT_ETH_SPEED_XLAUI_4_XLPPI_4_40G,
 		.mask_ethtool	= mlxsw_sp2_mask_ethtool_xlaui_4_xlppi_4_40g,
 		.m_ethtool_len	= MLXSW_SP2_MASK_ETHTOOL_XLAUI_4_XLPPI_4_40G_LEN,
-		.mask_width	= MLXSW_SP_PORT_MASK_WIDTH_4X |
+		.mask_sup_width	= MLXSW_SP_PORT_MASK_WIDTH_4X |
 				  MLXSW_SP_PORT_MASK_WIDTH_8X,
 		.speed		= SPEED_40000,
+		.width		= 4,
 	},
 	{
 		.mask		= MLXSW_REG_PTYS_EXT_ETH_SPEED_25GAUI_1_25GBASE_CR_KR,
 		.mask_ethtool	= mlxsw_sp2_mask_ethtool_25gaui_1_25gbase_cr_kr,
 		.m_ethtool_len	= MLXSW_SP2_MASK_ETHTOOL_25GAUI_1_25GBASE_CR_KR_LEN,
-		.mask_width	= MLXSW_SP_PORT_MASK_WIDTH_1X |
+		.mask_sup_width	= MLXSW_SP_PORT_MASK_WIDTH_1X |
 				  MLXSW_SP_PORT_MASK_WIDTH_2X |
 				  MLXSW_SP_PORT_MASK_WIDTH_4X |
 				  MLXSW_SP_PORT_MASK_WIDTH_8X,
 		.speed		= SPEED_25000,
+		.width		= 1,
 	},
 	{
 		.mask		= MLXSW_REG_PTYS_EXT_ETH_SPEED_50GAUI_2_LAUI_2_50GBASE_CR2_KR2,
 		.mask_ethtool	= mlxsw_sp2_mask_ethtool_50gaui_2_laui_2_50gbase_cr2_kr2,
 		.m_ethtool_len	= MLXSW_SP2_MASK_ETHTOOL_50GAUI_2_LAUI_2_50GBASE_CR2_KR2_LEN,
-		.mask_width	= MLXSW_SP_PORT_MASK_WIDTH_2X |
+		.mask_sup_width	= MLXSW_SP_PORT_MASK_WIDTH_2X |
 				  MLXSW_SP_PORT_MASK_WIDTH_4X |
 				  MLXSW_SP_PORT_MASK_WIDTH_8X,
 		.speed		= SPEED_50000,
+		.width		= 2,
 	},
 	{
 		.mask		= MLXSW_REG_PTYS_EXT_ETH_SPEED_50GAUI_1_LAUI_1_50GBASE_CR_KR,
 		.mask_ethtool	= mlxsw_sp2_mask_ethtool_50gaui_1_laui_1_50gbase_cr_kr,
 		.m_ethtool_len	= MLXSW_SP2_MASK_ETHTOOL_50GAUI_1_LAUI_1_50GBASE_CR_KR_LEN,
-		.mask_width	= MLXSW_SP_PORT_MASK_WIDTH_1X,
+		.mask_sup_width	= MLXSW_SP_PORT_MASK_WIDTH_1X,
 		.speed		= SPEED_50000,
+		.width		= 1,
 	},
 	{
 		.mask		= MLXSW_REG_PTYS_EXT_ETH_SPEED_CAUI_4_100GBASE_CR4_KR4,
 		.mask_ethtool	= mlxsw_sp2_mask_ethtool_caui_4_100gbase_cr4_kr4,
 		.m_ethtool_len	= MLXSW_SP2_MASK_ETHTOOL_CAUI_4_100GBASE_CR4_KR4_LEN,
-		.mask_width	= MLXSW_SP_PORT_MASK_WIDTH_4X |
+		.mask_sup_width	= MLXSW_SP_PORT_MASK_WIDTH_4X |
 				  MLXSW_SP_PORT_MASK_WIDTH_8X,
 		.speed		= SPEED_100000,
+		.width		= 4,
 	},
 	{
 		.mask		= MLXSW_REG_PTYS_EXT_ETH_SPEED_100GAUI_2_100GBASE_CR2_KR2,
 		.mask_ethtool	= mlxsw_sp2_mask_ethtool_100gaui_2_100gbase_cr2_kr2,
 		.m_ethtool_len	= MLXSW_SP2_MASK_ETHTOOL_100GAUI_2_100GBASE_CR2_KR2_LEN,
-		.mask_width	= MLXSW_SP_PORT_MASK_WIDTH_2X,
+		.mask_sup_width	= MLXSW_SP_PORT_MASK_WIDTH_2X,
 		.speed		= SPEED_100000,
+		.width		= 2,
 	},
 	{
 		.mask		= MLXSW_REG_PTYS_EXT_ETH_SPEED_200GAUI_4_200GBASE_CR4_KR4,
 		.mask_ethtool	= mlxsw_sp2_mask_ethtool_200gaui_4_200gbase_cr4_kr4,
 		.m_ethtool_len	= MLXSW_SP2_MASK_ETHTOOL_200GAUI_4_200GBASE_CR4_KR4_LEN,
-		.mask_width	= MLXSW_SP_PORT_MASK_WIDTH_4X |
+		.mask_sup_width	= MLXSW_SP_PORT_MASK_WIDTH_4X |
 				  MLXSW_SP_PORT_MASK_WIDTH_8X,
 		.speed		= SPEED_200000,
+		.width		= 4,
 	},
 	{
 		.mask		= MLXSW_REG_PTYS_EXT_ETH_SPEED_400GAUI_8,
 		.mask_ethtool	= mlxsw_sp2_mask_ethtool_400gaui_8,
 		.m_ethtool_len	= MLXSW_SP2_MASK_ETHTOOL_400GAUI_8_LEN,
-		.mask_width	= MLXSW_SP_PORT_MASK_WIDTH_8X,
+		.mask_sup_width	= MLXSW_SP_PORT_MASK_WIDTH_8X,
 		.speed		= SPEED_400000,
+		.width		= 8,
 	},
 };
 
@@ -1709,17 +1726,36 @@ mlxsw_sp2_to_ptys_advert_link(struct mlxsw_sp *mlxsw_sp,
 	return ptys_proto;
 }
 
-static u32 mlxsw_sp2_to_ptys_speed(struct mlxsw_sp *mlxsw_sp,
-				   u8 width, u32 speed)
+static u32 mlxsw_sp2_to_ptys_speed_lanes(struct mlxsw_sp *mlxsw_sp, u8 width,
+					 const struct ethtool_link_ksettings *cmd)
 {
 	u8 mask_width = mlxsw_sp_port_mask_width_get(width);
+	struct mlxsw_sp2_port_link_mode link_mode;
 	u32 ptys_proto = 0;
 	int i;
 
+	if (cmd->lanes > width)
+		return ptys_proto;
+
 	for (i = 0; i < MLXSW_SP2_PORT_LINK_MODE_LEN; i++) {
-		if ((speed == mlxsw_sp2_port_link_mode[i].speed) &&
-		    (mask_width & mlxsw_sp2_port_link_mode[i].mask_width))
-			ptys_proto |= mlxsw_sp2_port_link_mode[i].mask;
+		if (cmd->base.speed == mlxsw_sp2_port_link_mode[i].speed) {
+			link_mode = mlxsw_sp2_port_link_mode[i];
+
+			if (!cmd->lanes) {
+				/* If number of lanes was not set by user space,
+				 * choose the link mode that supports the width
+				 * of the port.
+				 */
+				if (mask_width & link_mode.mask_sup_width)
+					ptys_proto |= link_mode.mask;
+			} else if (cmd->lanes == link_mode.width) {
+				/* Else if the number of lanes was set, choose
+				 * the link mode that its actual width equals to
+				 * it.
+				 */
+				ptys_proto |= link_mode.mask;
+			}
+		}
 	}
 	return ptys_proto;
 }
@@ -1762,7 +1798,7 @@ const struct mlxsw_sp_port_type_speed_ops mlxsw_sp2_port_type_speed_ops = {
 	.from_ptys_speed_duplex		= mlxsw_sp2_from_ptys_speed_duplex,
 	.ptys_max_speed			= mlxsw_sp2_ptys_max_speed,
 	.to_ptys_advert_link		= mlxsw_sp2_to_ptys_advert_link,
-	.to_ptys_speed			= mlxsw_sp2_to_ptys_speed,
+	.to_ptys_speed_lanes		= mlxsw_sp2_to_ptys_speed_lanes,
 	.reg_ptys_eth_pack		= mlxsw_sp2_reg_ptys_eth_pack,
 	.reg_ptys_eth_unpack		= mlxsw_sp2_reg_ptys_eth_unpack,
 	.ptys_proto_cap_masked_get	= mlxsw_sp2_ptys_proto_cap_masked_get,
-- 
2.26.2


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

* [PATCH net-next v4 7/8] mlxsw: ethtool: Pass link mode in use to ethtool
  2021-02-02 18:06 [PATCH net-next v4 0/8] Support setting lanes via ethtool Danielle Ratson
                   ` (5 preceding siblings ...)
  2021-02-02 18:06 ` [PATCH net-next v4 6/8] mlxsw: ethtool: Add support for setting lanes when autoneg is off Danielle Ratson
@ 2021-02-02 18:06 ` Danielle Ratson
  2021-02-02 18:06 ` [PATCH net-next v4 8/8] net: selftests: Add lanes setting test Danielle Ratson
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Danielle Ratson @ 2021-02-02 18:06 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, jiri, andrew, f.fainelli, mkubecek, mlxsw, idosch,
	Danielle Ratson

Currently, when user space queries the link's parameters, as speed and
duplex, each parameter is passed from the driver to ethtool.

Instead, pass the link mode bit in use.
In Spectrum-1, simply pass the bit that is set to '1' from PTYS register.
In Spectrum-2, pass the first link mode bit in the mask of the used
link mode.

Signed-off-by: Danielle Ratson <danieller@nvidia.com>
---

Notes:
    v2:
    	* Reword commit message.
    	* Pass link mode bit to ethtool instead of link parameters.
    	* Use u32 for lanes param instead ETHTOOL_LANES defines.

 .../net/ethernet/mellanox/mlxsw/spectrum.h    |  6 +--
 .../mellanox/mlxsw/spectrum_ethtool.c         | 47 +++++++++++--------
 2 files changed, 30 insertions(+), 23 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
index 0ad6b8a581d5..4bf5c4ebc030 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
@@ -331,9 +331,9 @@ struct mlxsw_sp_port_type_speed_ops {
 	void (*from_ptys_link)(struct mlxsw_sp *mlxsw_sp, u32 ptys_eth_proto,
 			       unsigned long *mode);
 	u32 (*from_ptys_speed)(struct mlxsw_sp *mlxsw_sp, u32 ptys_eth_proto);
-	void (*from_ptys_speed_duplex)(struct mlxsw_sp *mlxsw_sp,
-				       bool carrier_ok, u32 ptys_eth_proto,
-				       struct ethtool_link_ksettings *cmd);
+	void (*from_ptys_link_mode)(struct mlxsw_sp *mlxsw_sp,
+				    bool carrier_ok, u32 ptys_eth_proto,
+				    struct ethtool_link_ksettings *cmd);
 	int (*ptys_max_speed)(struct mlxsw_sp_port *mlxsw_sp_port, u32 *p_max_speed);
 	u32 (*to_ptys_advert_link)(struct mlxsw_sp *mlxsw_sp,
 				   const struct ethtool_link_ksettings *cmd);
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_ethtool.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_ethtool.c
index b0031ae7e623..f3a7b8226912 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_ethtool.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_ethtool.c
@@ -966,8 +966,8 @@ static int mlxsw_sp_port_get_link_ksettings(struct net_device *dev,
 
 	cmd->base.autoneg = autoneg ? AUTONEG_ENABLE : AUTONEG_DISABLE;
 	cmd->base.port = mlxsw_sp_port_connector_port(connector_type);
-	ops->from_ptys_speed_duplex(mlxsw_sp, netif_carrier_ok(dev),
-				    eth_proto_oper, cmd);
+	ops->from_ptys_link_mode(mlxsw_sp, netif_carrier_ok(dev),
+				 eth_proto_oper, cmd);
 
 	return 0;
 }
@@ -1223,19 +1223,21 @@ mlxsw_sp1_from_ptys_speed(struct mlxsw_sp *mlxsw_sp, u32 ptys_eth_proto)
 }
 
 static void
-mlxsw_sp1_from_ptys_speed_duplex(struct mlxsw_sp *mlxsw_sp, bool carrier_ok,
-				 u32 ptys_eth_proto,
-				 struct ethtool_link_ksettings *cmd)
+mlxsw_sp1_from_ptys_link_mode(struct mlxsw_sp *mlxsw_sp, bool carrier_ok,
+			      u32 ptys_eth_proto,
+			      struct ethtool_link_ksettings *cmd)
 {
-	cmd->base.speed = SPEED_UNKNOWN;
-	cmd->base.duplex = DUPLEX_UNKNOWN;
+	int i;
+
+	cmd->link_mode = -1;
 
 	if (!carrier_ok)
 		return;
 
-	cmd->base.speed = mlxsw_sp1_from_ptys_speed(mlxsw_sp, ptys_eth_proto);
-	if (cmd->base.speed != SPEED_UNKNOWN)
-		cmd->base.duplex = DUPLEX_FULL;
+	for (i = 0; i < MLXSW_SP1_PORT_LINK_MODE_LEN; i++) {
+		if (ptys_eth_proto & mlxsw_sp1_port_link_mode[i].mask)
+			cmd->link_mode = mlxsw_sp1_port_link_mode[i].mask_ethtool;
+	}
 }
 
 static int mlxsw_sp1_ptys_max_speed(struct mlxsw_sp_port *mlxsw_sp_port, u32 *p_max_speed)
@@ -1324,7 +1326,7 @@ const struct mlxsw_sp_port_type_speed_ops mlxsw_sp1_port_type_speed_ops = {
 	.from_ptys_supported_port	= mlxsw_sp1_from_ptys_supported_port,
 	.from_ptys_link			= mlxsw_sp1_from_ptys_link,
 	.from_ptys_speed		= mlxsw_sp1_from_ptys_speed,
-	.from_ptys_speed_duplex		= mlxsw_sp1_from_ptys_speed_duplex,
+	.from_ptys_link_mode		= mlxsw_sp1_from_ptys_link_mode,
 	.ptys_max_speed			= mlxsw_sp1_ptys_max_speed,
 	.to_ptys_advert_link		= mlxsw_sp1_to_ptys_advert_link,
 	.to_ptys_speed_lanes		= mlxsw_sp1_to_ptys_speed_lanes,
@@ -1660,19 +1662,24 @@ mlxsw_sp2_from_ptys_speed(struct mlxsw_sp *mlxsw_sp, u32 ptys_eth_proto)
 }
 
 static void
-mlxsw_sp2_from_ptys_speed_duplex(struct mlxsw_sp *mlxsw_sp, bool carrier_ok,
-				 u32 ptys_eth_proto,
-				 struct ethtool_link_ksettings *cmd)
+mlxsw_sp2_from_ptys_link_mode(struct mlxsw_sp *mlxsw_sp, bool carrier_ok,
+			      u32 ptys_eth_proto,
+			      struct ethtool_link_ksettings *cmd)
 {
-	cmd->base.speed = SPEED_UNKNOWN;
-	cmd->base.duplex = DUPLEX_UNKNOWN;
+	struct mlxsw_sp2_port_link_mode link;
+	int i;
+
+	cmd->link_mode = -1;
 
 	if (!carrier_ok)
 		return;
 
-	cmd->base.speed = mlxsw_sp2_from_ptys_speed(mlxsw_sp, ptys_eth_proto);
-	if (cmd->base.speed != SPEED_UNKNOWN)
-		cmd->base.duplex = DUPLEX_FULL;
+	for (i = 0; i < MLXSW_SP2_PORT_LINK_MODE_LEN; i++) {
+		if (ptys_eth_proto & mlxsw_sp2_port_link_mode[i].mask) {
+			link = mlxsw_sp2_port_link_mode[i];
+			cmd->link_mode = link.mask_ethtool[1];
+		}
+	}
 }
 
 static int mlxsw_sp2_ptys_max_speed(struct mlxsw_sp_port *mlxsw_sp_port, u32 *p_max_speed)
@@ -1795,7 +1802,7 @@ const struct mlxsw_sp_port_type_speed_ops mlxsw_sp2_port_type_speed_ops = {
 	.from_ptys_supported_port	= mlxsw_sp2_from_ptys_supported_port,
 	.from_ptys_link			= mlxsw_sp2_from_ptys_link,
 	.from_ptys_speed		= mlxsw_sp2_from_ptys_speed,
-	.from_ptys_speed_duplex		= mlxsw_sp2_from_ptys_speed_duplex,
+	.from_ptys_link_mode		= mlxsw_sp2_from_ptys_link_mode,
 	.ptys_max_speed			= mlxsw_sp2_ptys_max_speed,
 	.to_ptys_advert_link		= mlxsw_sp2_to_ptys_advert_link,
 	.to_ptys_speed_lanes		= mlxsw_sp2_to_ptys_speed_lanes,
-- 
2.26.2


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

* [PATCH net-next v4 8/8] net: selftests: Add lanes setting test
  2021-02-02 18:06 [PATCH net-next v4 0/8] Support setting lanes via ethtool Danielle Ratson
                   ` (6 preceding siblings ...)
  2021-02-02 18:06 ` [PATCH net-next v4 7/8] mlxsw: ethtool: Pass link mode in use to ethtool Danielle Ratson
@ 2021-02-02 18:06 ` Danielle Ratson
  2021-02-02 19:54 ` [PATCH net-next v4 0/8] Support setting lanes via ethtool Edwin Peer
  2021-02-04  4:00 ` patchwork-bot+netdevbpf
  9 siblings, 0 replies; 14+ messages in thread
From: Danielle Ratson @ 2021-02-02 18:06 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, jiri, andrew, f.fainelli, mkubecek, mlxsw, idosch,
	Danielle Ratson

Test that setting lanes parameter is working.

Set max speed and max lanes in the list of advertised link modes,
and then try to set max speed with the lanes below max lanes if exists
in the list.

And then, test that setting number of lanes larger than max lanes fails.

Do the above for both autoneg on and off.

$ ./ethtool_lanes.sh

TEST: 4 lanes is autonegotiated                                     [ OK ]
TEST: Lanes number larger than max width is not set                 [ OK ]
TEST: Autoneg off, 4 lanes detected during force mode               [ OK ]
TEST: Lanes number larger than max width is not set                 [ OK ]

Signed-off-by: Danielle Ratson <danieller@nvidia.com>
---

Notes:
    v4:
    	* Change the check for lanes unsupported, to not having "Lanes"
    	  line at all.
    
    v3:
    	* Move the test to drivers/net/mlxsw.
    
    v2:
    	* Fix "then" to "than".
    	* Remove the test for recieving max_width when lanes is not set by
    	  user. When not setting lanes, we don't promise anything regarding
    	  what number of lanes will be chosen.
    	* Reword commit message.
    	* Reword the skip print when ethtool is old.

 .../drivers/net/mlxsw/ethtool_lanes.sh        | 187 ++++++++++++++++++
 .../selftests/net/forwarding/ethtool_lib.sh   |  34 ++++
 tools/testing/selftests/net/forwarding/lib.sh |  28 +++
 3 files changed, 249 insertions(+)
 create mode 100755 tools/testing/selftests/drivers/net/mlxsw/ethtool_lanes.sh

diff --git a/tools/testing/selftests/drivers/net/mlxsw/ethtool_lanes.sh b/tools/testing/selftests/drivers/net/mlxsw/ethtool_lanes.sh
new file mode 100755
index 000000000000..91891b9418d7
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/mlxsw/ethtool_lanes.sh
@@ -0,0 +1,187 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+
+lib_dir=$(dirname $0)/../../../net/forwarding
+
+ALL_TESTS="
+	autoneg
+	autoneg_force_mode
+"
+
+NUM_NETIFS=2
+: ${TIMEOUT:=30000} # ms
+source $lib_dir/lib.sh
+source $lib_dir/ethtool_lib.sh
+
+setup_prepare()
+{
+	swp1=${NETIFS[p1]}
+	swp2=${NETIFS[p2]}
+
+	ip link set dev $swp1 up
+	ip link set dev $swp2 up
+
+	busywait "$TIMEOUT" wait_for_port_up ethtool $swp2
+	check_err $? "ports did not come up"
+
+	local lanes_exist=$(ethtool $swp1 | grep 'Lanes:')
+	if [[ -z $lanes_exist ]]; then
+		log_test "SKIP: driver does not support lanes setting"
+		exit 1
+	fi
+
+	ip link set dev $swp2 down
+	ip link set dev $swp1 down
+}
+
+check_lanes()
+{
+	local dev=$1; shift
+	local lanes=$1; shift
+	local max_speed=$1; shift
+	local chosen_lanes
+
+	chosen_lanes=$(ethtool $dev | grep 'Lanes:')
+	chosen_lanes=${chosen_lanes#*"Lanes: "}
+
+	((chosen_lanes == lanes))
+	check_err $? "swp1 advertise $max_speed and $lanes, devs sync to $chosen_lanes"
+}
+
+check_unsupported_lanes()
+{
+	local dev=$1; shift
+	local max_speed=$1; shift
+	local max_lanes=$1; shift
+	local autoneg=$1; shift
+	local autoneg_str=""
+
+	local unsupported_lanes=$((max_lanes *= 2))
+
+	if [[ $autoneg -eq 0 ]]; then
+		autoneg_str="autoneg off"
+	fi
+
+	ethtool -s $swp1 speed $max_speed lanes $unsupported_lanes $autoneg_str &> /dev/null
+	check_fail $? "Unsuccessful $unsupported_lanes lanes setting was expected"
+}
+
+max_speed_and_lanes_get()
+{
+	local dev=$1; shift
+	local arr=("$@")
+	local max_lanes
+	local max_speed
+	local -a lanes_arr
+	local -a speeds_arr
+	local -a max_values
+
+	for ((i=0; i<${#arr[@]}; i+=2)); do
+		speeds_arr+=("${arr[$i]}")
+		lanes_arr+=("${arr[i+1]}")
+	done
+
+	max_values+=($(get_max "${speeds_arr[@]}"))
+	max_values+=($(get_max "${lanes_arr[@]}"))
+
+	echo ${max_values[@]}
+}
+
+search_linkmode()
+{
+	local speed=$1; shift
+	local lanes=$1; shift
+	local arr=("$@")
+
+	for ((i=0; i<${#arr[@]}; i+=2)); do
+		if [[ $speed -eq ${arr[$i]} && $lanes -eq ${arr[i+1]} ]]; then
+			return 1
+		fi
+	done
+	return 0
+}
+
+autoneg()
+{
+	RET=0
+
+	local lanes
+	local max_speed
+	local max_lanes
+
+	local -a linkmodes_params=($(dev_linkmodes_params_get $swp1 1))
+	local -a max_values=($(max_speed_and_lanes_get $swp1 "${linkmodes_params[@]}"))
+	max_speed=${max_values[0]}
+	max_lanes=${max_values[1]}
+
+	lanes=$max_lanes
+
+	while [[ $lanes -ge 1 ]]; do
+		search_linkmode $max_speed $lanes "${linkmodes_params[@]}"
+		if [[ $? -eq 1 ]]; then
+			ethtool_set $swp1 speed $max_speed lanes $lanes
+			ip link set dev $swp1 up
+			ip link set dev $swp2 up
+			busywait "$TIMEOUT" wait_for_port_up ethtool $swp2
+			check_err $? "ports did not come up"
+
+			check_lanes $swp1 $lanes $max_speed
+			log_test "$lanes lanes is autonegotiated"
+		fi
+		let $((lanes /= 2))
+	done
+
+	check_unsupported_lanes $swp1 $max_speed $max_lanes 1
+	log_test "Lanes number larger than max width is not set"
+
+	ip link set dev $swp2 down
+	ip link set dev $swp1 down
+}
+
+autoneg_force_mode()
+{
+	RET=0
+
+	local lanes
+	local max_speed
+	local max_lanes
+
+	local -a linkmodes_params=($(dev_linkmodes_params_get $swp1 1))
+	local -a max_values=($(max_speed_and_lanes_get $swp1 "${linkmodes_params[@]}"))
+	max_speed=${max_values[0]}
+	max_lanes=${max_values[1]}
+
+	lanes=$max_lanes
+
+	while [[ $lanes -ge 1 ]]; do
+		search_linkmode $max_speed $lanes "${linkmodes_params[@]}"
+		if [[ $? -eq 1 ]]; then
+			ethtool_set $swp1 speed $max_speed lanes $lanes autoneg off
+			ethtool_set $swp2 speed $max_speed lanes $lanes autoneg off
+			ip link set dev $swp1 up
+			ip link set dev $swp2 up
+			busywait "$TIMEOUT" wait_for_port_up ethtool $swp2
+			check_err $? "ports did not come up"
+
+			check_lanes $swp1 $lanes $max_speed
+			log_test "Autoneg off, $lanes lanes detected during force mode"
+		fi
+		let $((lanes /= 2))
+	done
+
+	check_unsupported_lanes $swp1 $max_speed $max_lanes 0
+	log_test "Lanes number larger than max width is not set"
+
+	ip link set dev $swp2 down
+	ip link set dev $swp1 down
+
+	ethtool -s $swp2 autoneg on
+	ethtool -s $swp1 autoneg on
+}
+
+check_ethtool_lanes_support
+setup_prepare
+
+tests_run
+
+exit $EXIT_STATUS
diff --git a/tools/testing/selftests/net/forwarding/ethtool_lib.sh b/tools/testing/selftests/net/forwarding/ethtool_lib.sh
index 9188e624dec0..b9bfb45085af 100644
--- a/tools/testing/selftests/net/forwarding/ethtool_lib.sh
+++ b/tools/testing/selftests/net/forwarding/ethtool_lib.sh
@@ -22,6 +22,40 @@ ethtool_set()
 	check_err $out "error in configuration. $cmd"
 }
 
+dev_linkmodes_params_get()
+{
+	local dev=$1; shift
+	local adver=$1; shift
+	local -a linkmodes_params
+	local param_count
+	local arr
+
+	if (($adver)); then
+		mode="Advertised link modes"
+	else
+		mode="Supported link modes"
+	fi
+
+	local -a dev_linkmodes=($(dev_speeds_get $dev 1 $adver))
+	for ((i=0; i<${#dev_linkmodes[@]}; i++)); do
+		linkmodes_params[$i]=$(echo -e "${dev_linkmodes[$i]}" | \
+			# Replaces all non numbers with spaces
+			sed -e 's/[^0-9]/ /g' | \
+			# Squeeze spaces in sequence to 1 space
+			tr -s ' ')
+		# Count how many numbers were found in the linkmode
+		param_count=$(echo "${linkmodes_params[$i]}" | wc -w)
+		if [[ $param_count -eq 1 ]]; then
+			linkmodes_params[$i]="${linkmodes_params[$i]} 1"
+		elif [[ $param_count -ge 3 ]]; then
+			arr=(${linkmodes_params[$i]})
+			# Take only first two params
+			linkmodes_params[$i]=$(echo "${arr[@]:0:2}")
+		fi
+	done
+	echo ${linkmodes_params[@]}
+}
+
 dev_speeds_get()
 {
 	local dev=$1; shift
diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
index 31ce478686cb..26cfc778ff26 100644
--- a/tools/testing/selftests/net/forwarding/lib.sh
+++ b/tools/testing/selftests/net/forwarding/lib.sh
@@ -69,6 +69,15 @@ check_tc_action_hw_stats_support()
 	fi
 }
 
+check_ethtool_lanes_support()
+{
+	ethtool --help 2>&1| grep lanes &> /dev/null
+	if [[ $? -ne 0 ]]; then
+		echo "SKIP: ethtool too old; it is missing lanes support"
+		exit 1
+	fi
+}
+
 if [[ "$(id -u)" -ne 0 ]]; then
 	echo "SKIP: need root privileges"
 	exit 0
@@ -263,6 +272,20 @@ not()
 	[[ $? != 0 ]]
 }
 
+get_max()
+{
+	local arr=("$@")
+
+	max=${arr[0]}
+	for cur in ${arr[@]}; do
+		if [[ $cur -gt $max ]]; then
+			max=$cur
+		fi
+	done
+
+	echo $max
+}
+
 grep_bridge_fdb()
 {
 	local addr=$1; shift
@@ -279,6 +302,11 @@ grep_bridge_fdb()
 	$@ | grep $addr | grep $flag "$word"
 }
 
+wait_for_port_up()
+{
+	"$@" | grep -q "Link detected: yes"
+}
+
 wait_for_offload()
 {
 	"$@" | grep -q offload
-- 
2.26.2


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

* Re: [PATCH net-next v4 0/8] Support setting lanes via ethtool
  2021-02-02 18:06 [PATCH net-next v4 0/8] Support setting lanes via ethtool Danielle Ratson
                   ` (7 preceding siblings ...)
  2021-02-02 18:06 ` [PATCH net-next v4 8/8] net: selftests: Add lanes setting test Danielle Ratson
@ 2021-02-02 19:54 ` Edwin Peer
  2021-02-04  4:00 ` patchwork-bot+netdevbpf
  9 siblings, 0 replies; 14+ messages in thread
From: Edwin Peer @ 2021-02-02 19:54 UTC (permalink / raw)
  To: Danielle Ratson
  Cc: netdev, David S . Miller, Jakub Kicinski, Jiri Pirko,
	Andrew Lunn, f.fainelli, Michal Kubecek, mlxsw, Ido Schimmel

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

On Tue, Feb 2, 2021 at 10:14 AM Danielle Ratson <danieller@nvidia.com> wrote:

> Danielle Ratson (8):
>   ethtool: Validate master slave configuration before rtnl_lock()
>   ethtool: Extend link modes settings uAPI with lanes
>   ethtool: Get link mode in use instead of speed and duplex parameters
>   ethtool: Expose the number of lanes in use
>   mlxsw: ethtool: Remove max lanes filtering
>   mlxsw: ethtool: Add support for setting lanes when autoneg is off
>   mlxsw: ethtool: Pass link mode in use to ethtool
>   net: selftests: Add lanes setting test

Reviewed by: Edwin Peer <edwin.peer@broadcom.com>

Regards,
Edwin Peer

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4160 bytes --]

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

* Re: [PATCH net-next v4 0/8] Support setting lanes via ethtool
  2021-02-02 18:06 [PATCH net-next v4 0/8] Support setting lanes via ethtool Danielle Ratson
                   ` (8 preceding siblings ...)
  2021-02-02 19:54 ` [PATCH net-next v4 0/8] Support setting lanes via ethtool Edwin Peer
@ 2021-02-04  4:00 ` patchwork-bot+netdevbpf
  9 siblings, 0 replies; 14+ messages in thread
From: patchwork-bot+netdevbpf @ 2021-02-04  4:00 UTC (permalink / raw)
  To: Danielle Ratson
  Cc: netdev, davem, kuba, jiri, andrew, f.fainelli, mkubecek, mlxsw, idosch

Hello:

This series was applied to netdev/net-next.git (refs/heads/master):

On Tue, 2 Feb 2021 20:06:04 +0200 you wrote:
> Some speeds can be achieved with different number of lanes. For example,
> 100Gbps can be achieved using two lanes of 50Gbps or four lanes of
> 25Gbps. This patchset adds a new selector that allows ethtool to
> advertise link modes according to their number of lanes and also force a
> specific number of lanes when autonegotiation is off.
> 
> Advertising all link modes with a speed of 100Gbps that use two lanes:
> 
> [...]

Here is the summary with links:
  - [net-next,v4,1/8] ethtool: Validate master slave configuration before rtnl_lock()
    https://git.kernel.org/netdev/net-next/c/189e7a8d9420
  - [net-next,v4,2/8] ethtool: Extend link modes settings uAPI with lanes
    https://git.kernel.org/netdev/net-next/c/012ce4dd3102
  - [net-next,v4,3/8] ethtool: Get link mode in use instead of speed and duplex parameters
    https://git.kernel.org/netdev/net-next/c/c8907043c6ac
  - [net-next,v4,4/8] ethtool: Expose the number of lanes in use
    https://git.kernel.org/netdev/net-next/c/7dc33f0914a9
  - [net-next,v4,5/8] mlxsw: ethtool: Remove max lanes filtering
    https://git.kernel.org/netdev/net-next/c/5fc4053df3d9
  - [net-next,v4,6/8] mlxsw: ethtool: Add support for setting lanes when autoneg is off
    https://git.kernel.org/netdev/net-next/c/763ece86f0c2
  - [net-next,v4,7/8] mlxsw: ethtool: Pass link mode in use to ethtool
    https://git.kernel.org/netdev/net-next/c/25a96f057a0f
  - [net-next,v4,8/8] net: selftests: Add lanes setting test
    https://git.kernel.org/netdev/net-next/c/f72e2f48c710

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



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

* Re: [PATCH net-next v4 3/8] ethtool: Get link mode in use instead of speed and duplex parameters
  2021-02-02 18:06 ` [PATCH net-next v4 3/8] ethtool: Get link mode in use instead of speed and duplex parameters Danielle Ratson
@ 2021-02-22  9:41   ` Eric Dumazet
  2021-02-22 13:11     ` Danielle Ratson
  2021-02-22 13:39     ` Andrew Lunn
  0 siblings, 2 replies; 14+ messages in thread
From: Eric Dumazet @ 2021-02-22  9:41 UTC (permalink / raw)
  To: Danielle Ratson, netdev
  Cc: davem, kuba, jiri, andrew, f.fainelli, mkubecek, mlxsw, idosch



On 2/2/21 7:06 PM, Danielle Ratson wrote:
> Currently, when user space queries the link's parameters, as speed and
> duplex, each parameter is passed from the driver to ethtool.
> 
> Instead, get the link mode bit in use, and derive each of the parameters
> from it in ethtool.
> 
> Signed-off-by: Danielle Ratson <danieller@nvidia.com>
> ---
> 
> Notes:
>     v3:
>     	* Remove 'ETHTOOL_A_LINKMODES_LINK_MODE' from Documentation since
>     	  it is not used.
>     	* Remove LINK_MODE_UNKNOWN from uapi.
>     	* Remove an unnecessary loop.
>     	* Move link_mode_info and link_mode_params to common file.
>     	* Move the speed, duplex and lanes derivation to the wrapper
>     	  __ethtool_get_link_ksettings().
> 
>  include/linux/ethtool.h |   1 +
>  net/ethtool/common.c    | 147 +++++++++++++++++++++++++++++++++++++
>  net/ethtool/common.h    |   7 ++
>  net/ethtool/ioctl.c     |  18 ++++-
>  net/ethtool/linkmodes.c | 157 +---------------------------------------
>  5 files changed, 174 insertions(+), 156 deletions(-)
> 
> diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
> index 1ab13c5dfb2f..ec4cd3921c67 100644
> --- a/include/linux/ethtool.h
> +++ b/include/linux/ethtool.h
> @@ -129,6 +129,7 @@ struct ethtool_link_ksettings {
>  		__ETHTOOL_DECLARE_LINK_MODE_MASK(lp_advertising);
>  	} link_modes;
>  	u32	lanes;
> +	enum ethtool_link_mode_bit_indices link_mode;

Some drivers blindly use ethtool_link_ksettings without
knowing a new field has been added.

(See below)

>  };
>  
>  /**
> diff --git a/net/ethtool/common.c b/net/ethtool/common.c
> index 181220101a6e..835b9bba3e7e 100644
> --- a/net/ethtool/common.c
> +++ b/net/ethtool/common.c
> @@ -198,6 +198,153 @@ const char link_mode_names[][ETH_GSTRING_LEN] = {
>  };
>  static_assert(ARRAY_SIZE(link_mode_names) == __ETHTOOL_LINK_MODE_MASK_NBITS);
>  
> +#define __LINK_MODE_LANES_CR		1
> +#define __LINK_MODE_LANES_CR2		2
> +#define __LINK_MODE_LANES_CR4		4
> +#define __LINK_MODE_LANES_CR8		8
> +#define __LINK_MODE_LANES_DR		1
> +#define __LINK_MODE_LANES_DR2		2
> +#define __LINK_MODE_LANES_DR4		4
> +#define __LINK_MODE_LANES_DR8		8
> +#define __LINK_MODE_LANES_KR		1
> +#define __LINK_MODE_LANES_KR2		2
> +#define __LINK_MODE_LANES_KR4		4
> +#define __LINK_MODE_LANES_KR8		8
> +#define __LINK_MODE_LANES_SR		1
> +#define __LINK_MODE_LANES_SR2		2
> +#define __LINK_MODE_LANES_SR4		4
> +#define __LINK_MODE_LANES_SR8		8
> +#define __LINK_MODE_LANES_ER		1
> +#define __LINK_MODE_LANES_KX		1
> +#define __LINK_MODE_LANES_KX4		4
> +#define __LINK_MODE_LANES_LR		1
> +#define __LINK_MODE_LANES_LR4		4
> +#define __LINK_MODE_LANES_LR4_ER4	4
> +#define __LINK_MODE_LANES_LR_ER_FR	1
> +#define __LINK_MODE_LANES_LR2_ER2_FR2	2
> +#define __LINK_MODE_LANES_LR4_ER4_FR4	4
> +#define __LINK_MODE_LANES_LR8_ER8_FR8	8
> +#define __LINK_MODE_LANES_LRM		1
> +#define __LINK_MODE_LANES_MLD2		2
> +#define __LINK_MODE_LANES_T		1
> +#define __LINK_MODE_LANES_T1		1
> +#define __LINK_MODE_LANES_X		1
> +#define __LINK_MODE_LANES_FX		1
> +
> +#define __DEFINE_LINK_MODE_PARAMS(_speed, _type, _duplex)	\
> +	[ETHTOOL_LINK_MODE(_speed, _type, _duplex)] = {		\
> +		.speed  = SPEED_ ## _speed, \
> +		.lanes  = __LINK_MODE_LANES_ ## _type, \
> +		.duplex	= __DUPLEX_ ## _duplex \
> +	}
> +#define __DUPLEX_Half DUPLEX_HALF
> +#define __DUPLEX_Full DUPLEX_FULL
> +#define __DEFINE_SPECIAL_MODE_PARAMS(_mode) \
> +	[ETHTOOL_LINK_MODE_ ## _mode ## _BIT] = { \
> +		.speed	= SPEED_UNKNOWN, \
> +		.lanes	= 0, \
> +		.duplex	= DUPLEX_UNKNOWN, \
> +	}
> +
> +const struct link_mode_info link_mode_params[] = {
> +	__DEFINE_LINK_MODE_PARAMS(10, T, Half),
> +	__DEFINE_LINK_MODE_PARAMS(10, T, Full),
> +	__DEFINE_LINK_MODE_PARAMS(100, T, Half),
> +	__DEFINE_LINK_MODE_PARAMS(100, T, Full),
> +	__DEFINE_LINK_MODE_PARAMS(1000, T, Half),
> +	__DEFINE_LINK_MODE_PARAMS(1000, T, Full),
> +	__DEFINE_SPECIAL_MODE_PARAMS(Autoneg),
> +	__DEFINE_SPECIAL_MODE_PARAMS(TP),
> +	__DEFINE_SPECIAL_MODE_PARAMS(AUI),
> +	__DEFINE_SPECIAL_MODE_PARAMS(MII),
> +	__DEFINE_SPECIAL_MODE_PARAMS(FIBRE),
> +	__DEFINE_SPECIAL_MODE_PARAMS(BNC),
> +	__DEFINE_LINK_MODE_PARAMS(10000, T, Full),
> +	__DEFINE_SPECIAL_MODE_PARAMS(Pause),
> +	__DEFINE_SPECIAL_MODE_PARAMS(Asym_Pause),
> +	__DEFINE_LINK_MODE_PARAMS(2500, X, Full),
> +	__DEFINE_SPECIAL_MODE_PARAMS(Backplane),
> +	__DEFINE_LINK_MODE_PARAMS(1000, KX, Full),
> +	__DEFINE_LINK_MODE_PARAMS(10000, KX4, Full),
> +	__DEFINE_LINK_MODE_PARAMS(10000, KR, Full),
> +	[ETHTOOL_LINK_MODE_10000baseR_FEC_BIT] = {
> +		.speed	= SPEED_10000,
> +		.duplex = DUPLEX_FULL,
> +	},
> +	__DEFINE_LINK_MODE_PARAMS(20000, MLD2, Full),
> +	__DEFINE_LINK_MODE_PARAMS(20000, KR2, Full),
> +	__DEFINE_LINK_MODE_PARAMS(40000, KR4, Full),
> +	__DEFINE_LINK_MODE_PARAMS(40000, CR4, Full),
> +	__DEFINE_LINK_MODE_PARAMS(40000, SR4, Full),
> +	__DEFINE_LINK_MODE_PARAMS(40000, LR4, Full),
> +	__DEFINE_LINK_MODE_PARAMS(56000, KR4, Full),
> +	__DEFINE_LINK_MODE_PARAMS(56000, CR4, Full),
> +	__DEFINE_LINK_MODE_PARAMS(56000, SR4, Full),
> +	__DEFINE_LINK_MODE_PARAMS(56000, LR4, Full),
> +	__DEFINE_LINK_MODE_PARAMS(25000, CR, Full),
> +	__DEFINE_LINK_MODE_PARAMS(25000, KR, Full),
> +	__DEFINE_LINK_MODE_PARAMS(25000, SR, Full),
> +	__DEFINE_LINK_MODE_PARAMS(50000, CR2, Full),
> +	__DEFINE_LINK_MODE_PARAMS(50000, KR2, Full),
> +	__DEFINE_LINK_MODE_PARAMS(100000, KR4, Full),
> +	__DEFINE_LINK_MODE_PARAMS(100000, SR4, Full),
> +	__DEFINE_LINK_MODE_PARAMS(100000, CR4, Full),
> +	__DEFINE_LINK_MODE_PARAMS(100000, LR4_ER4, Full),
> +	__DEFINE_LINK_MODE_PARAMS(50000, SR2, Full),
> +	__DEFINE_LINK_MODE_PARAMS(1000, X, Full),
> +	__DEFINE_LINK_MODE_PARAMS(10000, CR, Full),
> +	__DEFINE_LINK_MODE_PARAMS(10000, SR, Full),
> +	__DEFINE_LINK_MODE_PARAMS(10000, LR, Full),
> +	__DEFINE_LINK_MODE_PARAMS(10000, LRM, Full),
> +	__DEFINE_LINK_MODE_PARAMS(10000, ER, Full),
> +	__DEFINE_LINK_MODE_PARAMS(2500, T, Full),
> +	__DEFINE_LINK_MODE_PARAMS(5000, T, Full),
> +	__DEFINE_SPECIAL_MODE_PARAMS(FEC_NONE),
> +	__DEFINE_SPECIAL_MODE_PARAMS(FEC_RS),
> +	__DEFINE_SPECIAL_MODE_PARAMS(FEC_BASER),
> +	__DEFINE_LINK_MODE_PARAMS(50000, KR, Full),
> +	__DEFINE_LINK_MODE_PARAMS(50000, SR, Full),
> +	__DEFINE_LINK_MODE_PARAMS(50000, CR, Full),
> +	__DEFINE_LINK_MODE_PARAMS(50000, LR_ER_FR, Full),
> +	__DEFINE_LINK_MODE_PARAMS(50000, DR, Full),
> +	__DEFINE_LINK_MODE_PARAMS(100000, KR2, Full),
> +	__DEFINE_LINK_MODE_PARAMS(100000, SR2, Full),
> +	__DEFINE_LINK_MODE_PARAMS(100000, CR2, Full),
> +	__DEFINE_LINK_MODE_PARAMS(100000, LR2_ER2_FR2, Full),
> +	__DEFINE_LINK_MODE_PARAMS(100000, DR2, Full),
> +	__DEFINE_LINK_MODE_PARAMS(200000, KR4, Full),
> +	__DEFINE_LINK_MODE_PARAMS(200000, SR4, Full),
> +	__DEFINE_LINK_MODE_PARAMS(200000, LR4_ER4_FR4, Full),
> +	__DEFINE_LINK_MODE_PARAMS(200000, DR4, Full),
> +	__DEFINE_LINK_MODE_PARAMS(200000, CR4, Full),
> +	__DEFINE_LINK_MODE_PARAMS(100, T1, Full),
> +	__DEFINE_LINK_MODE_PARAMS(1000, T1, Full),
> +	__DEFINE_LINK_MODE_PARAMS(400000, KR8, Full),
> +	__DEFINE_LINK_MODE_PARAMS(400000, SR8, Full),
> +	__DEFINE_LINK_MODE_PARAMS(400000, LR8_ER8_FR8, Full),
> +	__DEFINE_LINK_MODE_PARAMS(400000, DR8, Full),
> +	__DEFINE_LINK_MODE_PARAMS(400000, CR8, Full),
> +	__DEFINE_SPECIAL_MODE_PARAMS(FEC_LLRS),
> +	__DEFINE_LINK_MODE_PARAMS(100000, KR, Full),
> +	__DEFINE_LINK_MODE_PARAMS(100000, SR, Full),
> +	__DEFINE_LINK_MODE_PARAMS(100000, LR_ER_FR, Full),
> +	__DEFINE_LINK_MODE_PARAMS(100000, DR, Full),
> +	__DEFINE_LINK_MODE_PARAMS(100000, CR, Full),
> +	__DEFINE_LINK_MODE_PARAMS(200000, KR2, Full),
> +	__DEFINE_LINK_MODE_PARAMS(200000, SR2, Full),
> +	__DEFINE_LINK_MODE_PARAMS(200000, LR2_ER2_FR2, Full),
> +	__DEFINE_LINK_MODE_PARAMS(200000, DR2, Full),
> +	__DEFINE_LINK_MODE_PARAMS(200000, CR2, Full),
> +	__DEFINE_LINK_MODE_PARAMS(400000, KR4, Full),
> +	__DEFINE_LINK_MODE_PARAMS(400000, SR4, Full),
> +	__DEFINE_LINK_MODE_PARAMS(400000, LR4_ER4_FR4, Full),
> +	__DEFINE_LINK_MODE_PARAMS(400000, DR4, Full),
> +	__DEFINE_LINK_MODE_PARAMS(400000, CR4, Full),
> +	__DEFINE_LINK_MODE_PARAMS(100, FX, Half),
> +	__DEFINE_LINK_MODE_PARAMS(100, FX, Full),
> +};
> +static_assert(ARRAY_SIZE(link_mode_params) == __ETHTOOL_LINK_MODE_MASK_NBITS);
> +
>  const char netif_msg_class_names[][ETH_GSTRING_LEN] = {
>  	[NETIF_MSG_DRV_BIT]		= "drv",
>  	[NETIF_MSG_PROBE_BIT]		= "probe",
> diff --git a/net/ethtool/common.h b/net/ethtool/common.h
> index 3d9251c95a8b..a9d071248698 100644
> --- a/net/ethtool/common.h
> +++ b/net/ethtool/common.h
> @@ -14,6 +14,12 @@
>  
>  #define __SOF_TIMESTAMPING_CNT (const_ilog2(SOF_TIMESTAMPING_LAST) + 1)
>  
> +struct link_mode_info {
> +	int				speed;
> +	u8				lanes;
> +	u8				duplex;
> +};
> +
>  extern const char
>  netdev_features_strings[NETDEV_FEATURE_COUNT][ETH_GSTRING_LEN];
>  extern const char
> @@ -23,6 +29,7 @@ tunable_strings[__ETHTOOL_TUNABLE_COUNT][ETH_GSTRING_LEN];
>  extern const char
>  phy_tunable_strings[__ETHTOOL_PHY_TUNABLE_COUNT][ETH_GSTRING_LEN];
>  extern const char link_mode_names[][ETH_GSTRING_LEN];
> +extern const struct link_mode_info link_mode_params[];
>  extern const char netif_msg_class_names[][ETH_GSTRING_LEN];
>  extern const char wol_mode_names[][ETH_GSTRING_LEN];
>  extern const char sof_timestamping_names[][ETH_GSTRING_LEN];
> diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
> index 771688e1b0da..24783b71c584 100644
> --- a/net/ethtool/ioctl.c
> +++ b/net/ethtool/ioctl.c
> @@ -426,13 +426,29 @@ struct ethtool_link_usettings {
>  int __ethtool_get_link_ksettings(struct net_device *dev,
>  				 struct ethtool_link_ksettings *link_ksettings)
>  {
> +	const struct link_mode_info *link_info;
> +	int err;
> +
>  	ASSERT_RTNL();
>  
>  	if (!dev->ethtool_ops->get_link_ksettings)
>  		return -EOPNOTSUPP;
>  
>  	memset(link_ksettings, 0, sizeof(*link_ksettings));
> -	return dev->ethtool_ops->get_link_ksettings(dev, link_ksettings);
> +
> +	link_ksettings->link_mode = -1;
> +	err = dev->ethtool_ops->get_link_ksettings(dev, link_ksettings);
> +	if (err)
> +		return err;

If a driver like drivers/net/tun.c does a complete

memcpy(cmd, &tun->link_ksettings, sizeof(*cmd)); 

then the link_ksettings->link_mode is overwritten with possible
garbage data.

> +
> +	if (link_ksettings->link_mode != -1) {
> +		link_info = &link_mode_params[link_ksettings->link_mode];
> +		link_ksettings->base.speed = link_info->speed;
> +		link_ksettings->lanes = link_info->lanes;
> +		link_ksettings->base.duplex = link_info->duplex;
> +	}
> +
> +	return 0;


general protection fault, probably for non-canonical address 0xdffffc00f14cc32c: 0000 [#1] PREEMPT SMP KASAN
KASAN: probably user-memory-access in range [0x000000078a661960-0x000000078a661967]
CPU: 0 PID: 8452 Comm: syz-executor360 Not tainted 5.11.0-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
RIP: 0010:__ethtool_get_link_ksettings+0x1a3/0x3a0 net/ethtool/ioctl.c:446
Code: b7 3e fa 83 fd ff 0f 84 30 01 00 00 e8 16 b0 3e fa 48 8d 3c ed 60 d5 69 8a 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48 c1 ea 03 <0f> b6 14 02 48 89 f8 83 e0 07 83 c0 03 38 d0 7c 08 84 d2 0f 85 b9
RSP: 0018:ffffc900019df7a0 EFLAGS: 00010202
RAX: dffffc0000000000 RBX: ffff888026136008 RCX: 0000000000000000
RDX: 00000000f14cc32c RSI: ffffffff873439ca RDI: 000000078a661960
RBP: 00000000ffff8880 R08: 00000000ffffffff R09: ffff88802613606f
R10: ffffffff873439bc R11: 0000000000000000 R12: 0000000000000000
R13: ffff88802613606c R14: ffff888011d0c210 R15: ffff888011d0c210
FS:  0000000000749300(0000) GS:ffff8880b9c00000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00000000004b60f0 CR3: 00000000185c2000 CR4: 00000000001506f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
 linkinfo_prepare_data+0xfd/0x280 net/ethtool/linkinfo.c:37
 ethnl_default_notify+0x1dc/0x630 net/ethtool/netlink.c:586
 ethtool_notify+0xbd/0x1f0 net/ethtool/netlink.c:656
 ethtool_set_link_ksettings+0x277/0x330 net/ethtool/ioctl.c:620
 dev_ethtool+0x2b35/0x45d0 net/ethtool/ioctl.c:2842
 dev_ioctl+0x463/0xb70 net/core/dev_ioctl.c:440
 sock_do_ioctl+0x148/0x2d0 net/socket.c:1060
 sock_ioctl+0x477/0x6a0 net/socket.c:1177
 vfs_ioctl fs/ioctl.c:48 [inline]
 __do_sys_ioctl fs/ioctl.c:753 [inline]
 __se_sys_ioctl fs/ioctl.c:739 [inline]
 __x64_sys_ioctl+0x193/0x200 fs/ioctl.c:739
 do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46
 entry_SYSCALL_64_after_hwframe+0x44/0xa9
RIP: 0033:0x440c79
Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 41 15 00 00 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 c0 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007ffe7a6e6fc8 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 0000000000440c79
RDX: 0000000020000380 RSI: 0000000000008946 RDI: 0000000000000004
RBP: 0000000000000000 R08: 0000000000f0b5ff R09: 0000000000f0b5ff
R10: 0000000000f0b5ff R11: 0000000000000246 R12: 00000000000109be
R13: 00007ffe7a6e6ff0 R14: 00007ffe7a6e6fe0 R15: 00007ffe7a6e6fd4
Modules linked in:
---[ end trace 881142e6c455c35b ]---
RIP: 0010:__ethtool_get_link_ksettings+0x1a3/0x3a0 net/ethtool/ioctl.c:446
Code: b7 3e fa 83 fd ff 0f 84 30 01 00 00 e8 16 b0 3e fa 48 8d 3c ed 60 d5 69 8a 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48 c1 ea 03 <0f> b6 14 02 48 89 f8 83 e0 07 83 c0 03 38 d0 7c 08 84 d2 0f 85 b9
RSP: 0018:ffffc900019df7a0 EFLAGS: 00010202
RAX: dffffc0000000000 RBX: ffff888026136008 RCX: 0000000000000000
RDX: 00000000f14cc32c RSI: ffffffff873439ca RDI: 000000078a661960
RBP: 00000000ffff8880 R08: 00000000ffffffff R09: ffff88802613606f
R10: ffffffff873439bc R11: 0000000000000000 R12: 0000000000000000
R13: ffff88802613606c R14: ffff888011d0c210 R15: ffff888011d0c210
FS:  0000000000749300(0000) GS:ffff8880b9d00000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f4f9803d058 CR3: 00000000185c2000 CR4: 00000000001506e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000



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

* RE: [PATCH net-next v4 3/8] ethtool: Get link mode in use instead of speed and duplex parameters
  2021-02-22  9:41   ` Eric Dumazet
@ 2021-02-22 13:11     ` Danielle Ratson
  2021-02-22 13:39     ` Andrew Lunn
  1 sibling, 0 replies; 14+ messages in thread
From: Danielle Ratson @ 2021-02-22 13:11 UTC (permalink / raw)
  To: Eric Dumazet, netdev
  Cc: davem, kuba, Jiri Pirko, andrew, f.fainelli, mkubecek, mlxsw,
	Ido Schimmel



> -----Original Message-----
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Sent: Monday, February 22, 2021 11:42 AM
> To: Danielle Ratson <danieller@nvidia.com>; netdev@vger.kernel.org
> Cc: davem@davemloft.net; kuba@kernel.org; Jiri Pirko <jiri@nvidia.com>; andrew@lunn.ch; f.fainelli@gmail.com;
> mkubecek@suse.cz; mlxsw <mlxsw@nvidia.com>; Ido Schimmel <idosch@nvidia.com>
> Subject: Re: [PATCH net-next v4 3/8] ethtool: Get link mode in use instead of speed and duplex parameters
> 
> 
> 
> On 2/2/21 7:06 PM, Danielle Ratson wrote:
> > Currently, when user space queries the link's parameters, as speed and
> > duplex, each parameter is passed from the driver to ethtool.
> >
> > Instead, get the link mode bit in use, and derive each of the
> > parameters from it in ethtool.
> >
> > Signed-off-by: Danielle Ratson <danieller@nvidia.com>
> > ---
> >
> > Notes:
> >     v3:
> >     	* Remove 'ETHTOOL_A_LINKMODES_LINK_MODE' from Documentation since
> >     	  it is not used.
> >     	* Remove LINK_MODE_UNKNOWN from uapi.
> >     	* Remove an unnecessary loop.
> >     	* Move link_mode_info and link_mode_params to common file.
> >     	* Move the speed, duplex and lanes derivation to the wrapper
> >     	  __ethtool_get_link_ksettings().
> >
> >  include/linux/ethtool.h |   1 +
> >  net/ethtool/common.c    | 147 +++++++++++++++++++++++++++++++++++++
> >  net/ethtool/common.h    |   7 ++
> >  net/ethtool/ioctl.c     |  18 ++++-
> >  net/ethtool/linkmodes.c | 157
> > +---------------------------------------
> >  5 files changed, 174 insertions(+), 156 deletions(-)
> >
> > diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h index
> > 1ab13c5dfb2f..ec4cd3921c67 100644
> > --- a/include/linux/ethtool.h
> > +++ b/include/linux/ethtool.h
> > @@ -129,6 +129,7 @@ struct ethtool_link_ksettings {
> >  		__ETHTOOL_DECLARE_LINK_MODE_MASK(lp_advertising);
> >  	} link_modes;
> >  	u32	lanes;
> > +	enum ethtool_link_mode_bit_indices link_mode;
> 
> Some drivers blindly use ethtool_link_ksettings without knowing a new field has been added.
> 
> (See below)
> 
> >  };
> >
> >  /**
> > diff --git a/net/ethtool/common.c b/net/ethtool/common.c index
> > 181220101a6e..835b9bba3e7e 100644
> > --- a/net/ethtool/common.c
> > +++ b/net/ethtool/common.c
> > @@ -198,6 +198,153 @@ const char link_mode_names[][ETH_GSTRING_LEN] =
> > {  };
> >  static_assert(ARRAY_SIZE(link_mode_names) ==
> > __ETHTOOL_LINK_MODE_MASK_NBITS);
> >
> > +#define __LINK_MODE_LANES_CR		1
> > +#define __LINK_MODE_LANES_CR2		2
> > +#define __LINK_MODE_LANES_CR4		4
> > +#define __LINK_MODE_LANES_CR8		8
> > +#define __LINK_MODE_LANES_DR		1
> > +#define __LINK_MODE_LANES_DR2		2
> > +#define __LINK_MODE_LANES_DR4		4
> > +#define __LINK_MODE_LANES_DR8		8
> > +#define __LINK_MODE_LANES_KR		1
> > +#define __LINK_MODE_LANES_KR2		2
> > +#define __LINK_MODE_LANES_KR4		4
> > +#define __LINK_MODE_LANES_KR8		8
> > +#define __LINK_MODE_LANES_SR		1
> > +#define __LINK_MODE_LANES_SR2		2
> > +#define __LINK_MODE_LANES_SR4		4
> > +#define __LINK_MODE_LANES_SR8		8
> > +#define __LINK_MODE_LANES_ER		1
> > +#define __LINK_MODE_LANES_KX		1
> > +#define __LINK_MODE_LANES_KX4		4
> > +#define __LINK_MODE_LANES_LR		1
> > +#define __LINK_MODE_LANES_LR4		4
> > +#define __LINK_MODE_LANES_LR4_ER4	4
> > +#define __LINK_MODE_LANES_LR_ER_FR	1
> > +#define __LINK_MODE_LANES_LR2_ER2_FR2	2
> > +#define __LINK_MODE_LANES_LR4_ER4_FR4	4
> > +#define __LINK_MODE_LANES_LR8_ER8_FR8	8
> > +#define __LINK_MODE_LANES_LRM		1
> > +#define __LINK_MODE_LANES_MLD2		2
> > +#define __LINK_MODE_LANES_T		1
> > +#define __LINK_MODE_LANES_T1		1
> > +#define __LINK_MODE_LANES_X		1
> > +#define __LINK_MODE_LANES_FX		1
> > +
> > +#define __DEFINE_LINK_MODE_PARAMS(_speed, _type, _duplex)	\
> > +	[ETHTOOL_LINK_MODE(_speed, _type, _duplex)] = {		\
> > +		.speed  = SPEED_ ## _speed, \
> > +		.lanes  = __LINK_MODE_LANES_ ## _type, \
> > +		.duplex	= __DUPLEX_ ## _duplex \
> > +	}
> > +#define __DUPLEX_Half DUPLEX_HALF
> > +#define __DUPLEX_Full DUPLEX_FULL
> > +#define __DEFINE_SPECIAL_MODE_PARAMS(_mode) \
> > +	[ETHTOOL_LINK_MODE_ ## _mode ## _BIT] = { \
> > +		.speed	= SPEED_UNKNOWN, \
> > +		.lanes	= 0, \
> > +		.duplex	= DUPLEX_UNKNOWN, \
> > +	}
> > +
> > +const struct link_mode_info link_mode_params[] = {
> > +	__DEFINE_LINK_MODE_PARAMS(10, T, Half),
> > +	__DEFINE_LINK_MODE_PARAMS(10, T, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(100, T, Half),
> > +	__DEFINE_LINK_MODE_PARAMS(100, T, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(1000, T, Half),
> > +	__DEFINE_LINK_MODE_PARAMS(1000, T, Full),
> > +	__DEFINE_SPECIAL_MODE_PARAMS(Autoneg),
> > +	__DEFINE_SPECIAL_MODE_PARAMS(TP),
> > +	__DEFINE_SPECIAL_MODE_PARAMS(AUI),
> > +	__DEFINE_SPECIAL_MODE_PARAMS(MII),
> > +	__DEFINE_SPECIAL_MODE_PARAMS(FIBRE),
> > +	__DEFINE_SPECIAL_MODE_PARAMS(BNC),
> > +	__DEFINE_LINK_MODE_PARAMS(10000, T, Full),
> > +	__DEFINE_SPECIAL_MODE_PARAMS(Pause),
> > +	__DEFINE_SPECIAL_MODE_PARAMS(Asym_Pause),
> > +	__DEFINE_LINK_MODE_PARAMS(2500, X, Full),
> > +	__DEFINE_SPECIAL_MODE_PARAMS(Backplane),
> > +	__DEFINE_LINK_MODE_PARAMS(1000, KX, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(10000, KX4, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(10000, KR, Full),
> > +	[ETHTOOL_LINK_MODE_10000baseR_FEC_BIT] = {
> > +		.speed	= SPEED_10000,
> > +		.duplex = DUPLEX_FULL,
> > +	},
> > +	__DEFINE_LINK_MODE_PARAMS(20000, MLD2, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(20000, KR2, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(40000, KR4, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(40000, CR4, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(40000, SR4, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(40000, LR4, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(56000, KR4, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(56000, CR4, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(56000, SR4, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(56000, LR4, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(25000, CR, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(25000, KR, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(25000, SR, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(50000, CR2, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(50000, KR2, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(100000, KR4, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(100000, SR4, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(100000, CR4, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(100000, LR4_ER4, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(50000, SR2, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(1000, X, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(10000, CR, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(10000, SR, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(10000, LR, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(10000, LRM, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(10000, ER, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(2500, T, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(5000, T, Full),
> > +	__DEFINE_SPECIAL_MODE_PARAMS(FEC_NONE),
> > +	__DEFINE_SPECIAL_MODE_PARAMS(FEC_RS),
> > +	__DEFINE_SPECIAL_MODE_PARAMS(FEC_BASER),
> > +	__DEFINE_LINK_MODE_PARAMS(50000, KR, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(50000, SR, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(50000, CR, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(50000, LR_ER_FR, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(50000, DR, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(100000, KR2, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(100000, SR2, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(100000, CR2, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(100000, LR2_ER2_FR2, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(100000, DR2, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(200000, KR4, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(200000, SR4, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(200000, LR4_ER4_FR4, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(200000, DR4, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(200000, CR4, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(100, T1, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(1000, T1, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(400000, KR8, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(400000, SR8, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(400000, LR8_ER8_FR8, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(400000, DR8, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(400000, CR8, Full),
> > +	__DEFINE_SPECIAL_MODE_PARAMS(FEC_LLRS),
> > +	__DEFINE_LINK_MODE_PARAMS(100000, KR, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(100000, SR, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(100000, LR_ER_FR, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(100000, DR, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(100000, CR, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(200000, KR2, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(200000, SR2, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(200000, LR2_ER2_FR2, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(200000, DR2, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(200000, CR2, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(400000, KR4, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(400000, SR4, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(400000, LR4_ER4_FR4, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(400000, DR4, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(400000, CR4, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(100, FX, Half),
> > +	__DEFINE_LINK_MODE_PARAMS(100, FX, Full), };
> > +static_assert(ARRAY_SIZE(link_mode_params) ==
> > +__ETHTOOL_LINK_MODE_MASK_NBITS);
> > +
> >  const char netif_msg_class_names[][ETH_GSTRING_LEN] = {
> >  	[NETIF_MSG_DRV_BIT]		= "drv",
> >  	[NETIF_MSG_PROBE_BIT]		= "probe",
> > diff --git a/net/ethtool/common.h b/net/ethtool/common.h index
> > 3d9251c95a8b..a9d071248698 100644
> > --- a/net/ethtool/common.h
> > +++ b/net/ethtool/common.h
> > @@ -14,6 +14,12 @@
> >
> >  #define __SOF_TIMESTAMPING_CNT (const_ilog2(SOF_TIMESTAMPING_LAST) +
> > 1)
> >
> > +struct link_mode_info {
> > +	int				speed;
> > +	u8				lanes;
> > +	u8				duplex;
> > +};
> > +
> >  extern const char
> >  netdev_features_strings[NETDEV_FEATURE_COUNT][ETH_GSTRING_LEN];
> >  extern const char
> > @@ -23,6 +29,7 @@
> > tunable_strings[__ETHTOOL_TUNABLE_COUNT][ETH_GSTRING_LEN];
> >  extern const char
> >  phy_tunable_strings[__ETHTOOL_PHY_TUNABLE_COUNT][ETH_GSTRING_LEN];
> >  extern const char link_mode_names[][ETH_GSTRING_LEN];
> > +extern const struct link_mode_info link_mode_params[];
> >  extern const char netif_msg_class_names[][ETH_GSTRING_LEN];
> >  extern const char wol_mode_names[][ETH_GSTRING_LEN];
> >  extern const char sof_timestamping_names[][ETH_GSTRING_LEN];
> > diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c index
> > 771688e1b0da..24783b71c584 100644
> > --- a/net/ethtool/ioctl.c
> > +++ b/net/ethtool/ioctl.c
> > @@ -426,13 +426,29 @@ struct ethtool_link_usettings {  int
> > __ethtool_get_link_ksettings(struct net_device *dev,
> >  				 struct ethtool_link_ksettings *link_ksettings)  {
> > +	const struct link_mode_info *link_info;
> > +	int err;
> > +
> >  	ASSERT_RTNL();
> >
> >  	if (!dev->ethtool_ops->get_link_ksettings)
> >  		return -EOPNOTSUPP;
> >
> >  	memset(link_ksettings, 0, sizeof(*link_ksettings));
> > -	return dev->ethtool_ops->get_link_ksettings(dev, link_ksettings);
> > +
> > +	link_ksettings->link_mode = -1;
> > +	err = dev->ethtool_ops->get_link_ksettings(dev, link_ksettings);
> > +	if (err)
> > +		return err;
> 
> If a driver like drivers/net/tun.c does a complete
> 
> memcpy(cmd, &tun->link_ksettings, sizeof(*cmd));
> 
> then the link_ksettings->link_mode is overwritten with possible garbage data.
> 
> > +
> > +	if (link_ksettings->link_mode != -1) {
> > +		link_info = &link_mode_params[link_ksettings->link_mode];
> > +		link_ksettings->base.speed = link_info->speed;
> > +		link_ksettings->lanes = link_info->lanes;
> > +		link_ksettings->base.duplex = link_info->duplex;
> > +	}
> > +
> > +	return 0;
> 
> 
> general protection fault, probably for non-canonical address 0xdffffc00f14cc32c: 0000 [#1] PREEMPT SMP KASAN
> KASAN: probably user-memory-access in range [0x000000078a661960-0x000000078a661967]
> CPU: 0 PID: 8452 Comm: syz-executor360 Not tainted 5.11.0-syzkaller #0 Hardware name: Google Google Compute Engine/Google
> Compute Engine, BIOS Google 01/01/2011
> RIP: 0010:__ethtool_get_link_ksettings+0x1a3/0x3a0 net/ethtool/ioctl.c:446
> Code: b7 3e fa 83 fd ff 0f 84 30 01 00 00 e8 16 b0 3e fa 48 8d 3c ed 60 d5 69 8a 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48 c1 ea 03 <0f> b6 14
> 02 48 89 f8 83 e0 07 83 c0 03 38 d0 7c 08 84 d2 0f 85 b9
> RSP: 0018:ffffc900019df7a0 EFLAGS: 00010202
> RAX: dffffc0000000000 RBX: ffff888026136008 RCX: 0000000000000000
> RDX: 00000000f14cc32c RSI: ffffffff873439ca RDI: 000000078a661960
> RBP: 00000000ffff8880 R08: 00000000ffffffff R09: ffff88802613606f
> R10: ffffffff873439bc R11: 0000000000000000 R12: 0000000000000000
> R13: ffff88802613606c R14: ffff888011d0c210 R15: ffff888011d0c210
> FS:  0000000000749300(0000) GS:ffff8880b9c00000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 00000000004b60f0 CR3: 00000000185c2000 CR4: 00000000001506f0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 Call Trace:
>  linkinfo_prepare_data+0xfd/0x280 net/ethtool/linkinfo.c:37
>  ethnl_default_notify+0x1dc/0x630 net/ethtool/netlink.c:586
>  ethtool_notify+0xbd/0x1f0 net/ethtool/netlink.c:656
>  ethtool_set_link_ksettings+0x277/0x330 net/ethtool/ioctl.c:620
>  dev_ethtool+0x2b35/0x45d0 net/ethtool/ioctl.c:2842
>  dev_ioctl+0x463/0xb70 net/core/dev_ioctl.c:440
>  sock_do_ioctl+0x148/0x2d0 net/socket.c:1060
>  sock_ioctl+0x477/0x6a0 net/socket.c:1177  vfs_ioctl fs/ioctl.c:48 [inline]  __do_sys_ioctl fs/ioctl.c:753 [inline]  __se_sys_ioctl
> fs/ioctl.c:739 [inline]
>  __x64_sys_ioctl+0x193/0x200 fs/ioctl.c:739
>  do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46
>  entry_SYSCALL_64_after_hwframe+0x44/0xa9
> RIP: 0033:0x440c79
> Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 41 15 00 00 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01
> f0 ff ff 73 01 c3 48 c7 c1 c0 ff ff ff f7 d8 64 89 01 48
> RSP: 002b:00007ffe7a6e6fc8 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
> RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 0000000000440c79
> RDX: 0000000020000380 RSI: 0000000000008946 RDI: 0000000000000004
> RBP: 0000000000000000 R08: 0000000000f0b5ff R09: 0000000000f0b5ff
> R10: 0000000000f0b5ff R11: 0000000000000246 R12: 00000000000109be
> R13: 00007ffe7a6e6ff0 R14: 00007ffe7a6e6fe0 R15: 00007ffe7a6e6fd4 Modules linked in:
> ---[ end trace 881142e6c455c35b ]---
> RIP: 0010:__ethtool_get_link_ksettings+0x1a3/0x3a0 net/ethtool/ioctl.c:446
> Code: b7 3e fa 83 fd ff 0f 84 30 01 00 00 e8 16 b0 3e fa 48 8d 3c ed 60 d5 69 8a 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48 c1 ea 03 <0f> b6 14
> 02 48 89 f8 83 e0 07 83 c0 03 38 d0 7c 08 84 d2 0f 85 b9
> RSP: 0018:ffffc900019df7a0 EFLAGS: 00010202
> RAX: dffffc0000000000 RBX: ffff888026136008 RCX: 0000000000000000
> RDX: 00000000f14cc32c RSI: ffffffff873439ca RDI: 000000078a661960
> RBP: 00000000ffff8880 R08: 00000000ffffffff R09: ffff88802613606f
> R10: ffffffff873439bc R11: 0000000000000000 R12: 0000000000000000
> R13: ffff88802613606c R14: ffff888011d0c210 R15: ffff888011d0c210
> FS:  0000000000749300(0000) GS:ffff8880b9d00000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 00007f4f9803d058 CR3: 00000000185c2000 CR4: 00000000001506e0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> 

Thank you for you catch Eric.

Ill go over all the get_link_ksettings functions and adjust them so link_mode will be equal to -1 instead of garbage value, in cases like you mentioned.


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

* Re: [PATCH net-next v4 3/8] ethtool: Get link mode in use instead of speed and duplex parameters
  2021-02-22  9:41   ` Eric Dumazet
  2021-02-22 13:11     ` Danielle Ratson
@ 2021-02-22 13:39     ` Andrew Lunn
  1 sibling, 0 replies; 14+ messages in thread
From: Andrew Lunn @ 2021-02-22 13:39 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Danielle Ratson, netdev, davem, kuba, jiri, f.fainelli, mkubecek,
	mlxsw, idosch

> > Currently, when user space queries the link's parameters, as speed and
> > duplex, each parameter is passed from the driver to ethtool.
> > 
> > Instead, get the link mode bit in use, and derive each of the parameters
> > from it in ethtool.

> > +	err = dev->ethtool_ops->get_link_ksettings(dev, link_ksettings);
> > +	if (err)
> > +		return err;
> 
> If a driver like drivers/net/tun.c does a complete
> 
> memcpy(cmd, &tun->link_ksettings, sizeof(*cmd)); 
> 
> then the link_ksettings->link_mode is overwritten with possible
> garbage data.
> 
> > +
> > +	if (link_ksettings->link_mode != -1) {
> > +		link_info = &link_mode_params[link_ksettings->link_mode];
> > +		link_ksettings->base.speed = link_info->speed;
> > +		link_ksettings->lanes = link_info->lanes;
> > +		link_ksettings->base.duplex = link_info->duplex;
> > +	}

Sorry, i missed the first posting of this.

What about downshift? A 1G PHY detects that it cannot establish a link
using four pairs at 1G. So it downshifts to 100Mbps using 2 pairs. The
PHY will report a speed of SPEED_100, despite the mode being
1000Base-T. This is not part of 802.3 clause 22, but a number of PHYs
have vendor registers which report the actual speed, and drivers are
reading this actual speed and returning it.

I really think you need to only use the link_mode derived speed when
speed is SPEED_UNKNOWN, duplex is DUPLEX_UNKNOWN.

	Andrew

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

end of thread, other threads:[~2021-02-22 13:44 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-02 18:06 [PATCH net-next v4 0/8] Support setting lanes via ethtool Danielle Ratson
2021-02-02 18:06 ` [PATCH net-next v4 1/8] ethtool: Validate master slave configuration before rtnl_lock() Danielle Ratson
2021-02-02 18:06 ` [PATCH net-next v4 2/8] ethtool: Extend link modes settings uAPI with lanes Danielle Ratson
2021-02-02 18:06 ` [PATCH net-next v4 3/8] ethtool: Get link mode in use instead of speed and duplex parameters Danielle Ratson
2021-02-22  9:41   ` Eric Dumazet
2021-02-22 13:11     ` Danielle Ratson
2021-02-22 13:39     ` Andrew Lunn
2021-02-02 18:06 ` [PATCH net-next v4 4/8] ethtool: Expose the number of lanes in use Danielle Ratson
2021-02-02 18:06 ` [PATCH net-next v4 5/8] mlxsw: ethtool: Remove max lanes filtering Danielle Ratson
2021-02-02 18:06 ` [PATCH net-next v4 6/8] mlxsw: ethtool: Add support for setting lanes when autoneg is off Danielle Ratson
2021-02-02 18:06 ` [PATCH net-next v4 7/8] mlxsw: ethtool: Pass link mode in use to ethtool Danielle Ratson
2021-02-02 18:06 ` [PATCH net-next v4 8/8] net: selftests: Add lanes setting test Danielle Ratson
2021-02-02 19:54 ` [PATCH net-next v4 0/8] Support setting lanes via ethtool Edwin Peer
2021-02-04  4:00 ` patchwork-bot+netdevbpf

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.