* [PATCH net-next v3 0/7] Support setting lanes via ethtool
@ 2021-01-20 9:37 Danielle Ratson
2021-01-20 9:37 ` [PATCH net-next v3 1/7] ethtool: Extend link modes settings uAPI with lanes Danielle Ratson
` (6 more replies)
0 siblings, 7 replies; 31+ messages in thread
From: Danielle Ratson @ 2021-01-20 9:37 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 allows user space to configure the desired number of lanes.
Patch #2-#3 adjusts ethtool to dump to user space the number of lanes
currently in use.
Patches #4-#6 add support for lanes configuration in mlxsw.
Patch #7 adds a selftest.
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 (7):
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.h | 2 +
include/uapi/linux/ethtool_netlink.h | 1 +
net/ethtool/common.c | 114 ++++++++++
net/ethtool/common.h | 7 +
net/ethtool/ioctl.c | 18 +-
net/ethtool/linkmodes.c | 179 +++++-----------
net/ethtool/netlink.h | 2 +-
.../drivers/net/mlxsw/ethtool_lanes.sh | 188 +++++++++++++++++
.../selftests/net/forwarding/ethtool_lib.sh | 34 +++
tools/testing/selftests/net/forwarding/lib.sh | 28 +++
14 files changed, 576 insertions(+), 222 deletions(-)
create mode 100755 tools/testing/selftests/drivers/net/mlxsw/ethtool_lanes.sh
--
2.26.2
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH net-next v3 1/7] ethtool: Extend link modes settings uAPI with lanes
2021-01-20 9:37 [PATCH net-next v3 0/7] Support setting lanes via ethtool Danielle Ratson
@ 2021-01-20 9:37 ` Danielle Ratson
2021-01-20 22:35 ` Edwin Peer
2021-01-22 3:44 ` Jakub Kicinski
2021-01-20 9:37 ` [PATCH net-next v3 2/7] ethtool: Get link mode in use instead of speed and duplex parameters Danielle Ratson
` (5 subsequent siblings)
6 siblings, 2 replies; 31+ messages in thread
From: Danielle Ratson @ 2021-01-20 9:37 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:
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.h | 2 +
include/uapi/linux/ethtool_netlink.h | 1 +
net/ethtool/linkmodes.c | 214 +++++++++++--------
net/ethtool/netlink.h | 2 +-
6 files changed, 139 insertions(+), 95 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.h b/include/uapi/linux/ethtool.h
index cde753bb2093..80edae2c24f7 100644
--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h
@@ -1738,6 +1738,8 @@ static inline int ethtool_validate_speed(__u32 speed)
return speed <= INT_MAX || speed == (__u32)SPEED_UNKNOWN;
}
+#define ETHTOOL_LANES_UNKNOWN 0
+
/* Duplex, half or full. */
#define DUPLEX_HALF 0x00
#define DUPLEX_FULL 0x01
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 c5bcb9abc8b9..fb7d73250864 100644
--- a/net/ethtool/linkmodes.c
+++ b/net/ethtool/linkmodes.c
@@ -152,12 +152,14 @@ const struct ethnl_request_ops ethnl_linkmodes_request_ops = {
struct link_mode_info {
int speed;
+ u32 lanes;
u8 duplex;
};
-#define __DEFINE_LINK_MODE_PARAMS(_speed, _type, _duplex) \
+#define __DEFINE_LINK_MODE_PARAMS(_speed, _type, _lanes, _duplex) \
[ETHTOOL_LINK_MODE(_speed, _type, _duplex)] = { \
.speed = SPEED_ ## _speed, \
+ .lanes = _lanes, \
.duplex = __DUPLEX_ ## _duplex \
}
#define __DUPLEX_Half DUPLEX_HALF
@@ -165,105 +167,106 @@ struct link_mode_info {
#define __DEFINE_SPECIAL_MODE_PARAMS(_mode) \
[ETHTOOL_LINK_MODE_ ## _mode ## _BIT] = { \
.speed = SPEED_UNKNOWN, \
+ .lanes = ETHTOOL_LANES_UNKNOWN, \
.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_LINK_MODE_PARAMS(10, T, 1, Half),
+ __DEFINE_LINK_MODE_PARAMS(10, T, 1, Full),
+ __DEFINE_LINK_MODE_PARAMS(100, T, 1, Half),
+ __DEFINE_LINK_MODE_PARAMS(100, T, 1, Full),
+ __DEFINE_LINK_MODE_PARAMS(1000, T, 1, Half),
+ __DEFINE_LINK_MODE_PARAMS(1000, T, 1, 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_LINK_MODE_PARAMS(10000, T, 1, Full),
__DEFINE_SPECIAL_MODE_PARAMS(Pause),
__DEFINE_SPECIAL_MODE_PARAMS(Asym_Pause),
- __DEFINE_LINK_MODE_PARAMS(2500, X, Full),
+ __DEFINE_LINK_MODE_PARAMS(2500, X, 1, 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),
+ __DEFINE_LINK_MODE_PARAMS(1000, KX, 1, Full),
+ __DEFINE_LINK_MODE_PARAMS(10000, KX4, 4, Full),
+ __DEFINE_LINK_MODE_PARAMS(10000, KR, 1, 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_LINK_MODE_PARAMS(20000, MLD2, 2, Full),
+ __DEFINE_LINK_MODE_PARAMS(20000, KR2, 2, Full),
+ __DEFINE_LINK_MODE_PARAMS(40000, KR4, 4, Full),
+ __DEFINE_LINK_MODE_PARAMS(40000, CR4, 4, Full),
+ __DEFINE_LINK_MODE_PARAMS(40000, SR4, 4, Full),
+ __DEFINE_LINK_MODE_PARAMS(40000, LR4, 4, Full),
+ __DEFINE_LINK_MODE_PARAMS(56000, KR4, 4, Full),
+ __DEFINE_LINK_MODE_PARAMS(56000, CR4, 4, Full),
+ __DEFINE_LINK_MODE_PARAMS(56000, SR4, 4, Full),
+ __DEFINE_LINK_MODE_PARAMS(56000, LR4, 4, Full),
+ __DEFINE_LINK_MODE_PARAMS(25000, CR, 1, Full),
+ __DEFINE_LINK_MODE_PARAMS(25000, KR, 1, Full),
+ __DEFINE_LINK_MODE_PARAMS(25000, SR, 1, Full),
+ __DEFINE_LINK_MODE_PARAMS(50000, CR2, 2, Full),
+ __DEFINE_LINK_MODE_PARAMS(50000, KR2, 2, Full),
+ __DEFINE_LINK_MODE_PARAMS(100000, KR4, 4, Full),
+ __DEFINE_LINK_MODE_PARAMS(100000, SR4, 4, Full),
+ __DEFINE_LINK_MODE_PARAMS(100000, CR4, 4, Full),
+ __DEFINE_LINK_MODE_PARAMS(100000, LR4_ER4, 4, Full),
+ __DEFINE_LINK_MODE_PARAMS(50000, SR2, 2, Full),
+ __DEFINE_LINK_MODE_PARAMS(1000, X, 1, Full),
+ __DEFINE_LINK_MODE_PARAMS(10000, CR, 1, Full),
+ __DEFINE_LINK_MODE_PARAMS(10000, SR, 1, Full),
+ __DEFINE_LINK_MODE_PARAMS(10000, LR, 1, Full),
+ __DEFINE_LINK_MODE_PARAMS(10000, LRM, 1, Full),
+ __DEFINE_LINK_MODE_PARAMS(10000, ER, 1, Full),
+ __DEFINE_LINK_MODE_PARAMS(2500, T, 1, Full),
+ __DEFINE_LINK_MODE_PARAMS(5000, T, 1, 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_LINK_MODE_PARAMS(50000, KR, 1, Full),
+ __DEFINE_LINK_MODE_PARAMS(50000, SR, 1, Full),
+ __DEFINE_LINK_MODE_PARAMS(50000, CR, 1, Full),
+ __DEFINE_LINK_MODE_PARAMS(50000, LR_ER_FR, 1, Full),
+ __DEFINE_LINK_MODE_PARAMS(50000, DR, 1, Full),
+ __DEFINE_LINK_MODE_PARAMS(100000, KR2, 2, Full),
+ __DEFINE_LINK_MODE_PARAMS(100000, SR2, 2, Full),
+ __DEFINE_LINK_MODE_PARAMS(100000, CR2, 2, Full),
+ __DEFINE_LINK_MODE_PARAMS(100000, LR2_ER2_FR2, 2, Full),
+ __DEFINE_LINK_MODE_PARAMS(100000, DR2, 2, Full),
+ __DEFINE_LINK_MODE_PARAMS(200000, KR4, 4, Full),
+ __DEFINE_LINK_MODE_PARAMS(200000, SR4, 4, Full),
+ __DEFINE_LINK_MODE_PARAMS(200000, LR4_ER4_FR4, 4, Full),
+ __DEFINE_LINK_MODE_PARAMS(200000, DR4, 4, Full),
+ __DEFINE_LINK_MODE_PARAMS(200000, CR4, 4, Full),
+ __DEFINE_LINK_MODE_PARAMS(100, T1, 1, Full),
+ __DEFINE_LINK_MODE_PARAMS(1000, T1, 1, Full),
+ __DEFINE_LINK_MODE_PARAMS(400000, KR8, 8, Full),
+ __DEFINE_LINK_MODE_PARAMS(400000, SR8, 8, Full),
+ __DEFINE_LINK_MODE_PARAMS(400000, LR8_ER8_FR8, 8, Full),
+ __DEFINE_LINK_MODE_PARAMS(400000, DR8, 8, Full),
+ __DEFINE_LINK_MODE_PARAMS(400000, CR8, 8, 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),
+ __DEFINE_LINK_MODE_PARAMS(100000, KR, 1, Full),
+ __DEFINE_LINK_MODE_PARAMS(100000, SR, 1, Full),
+ __DEFINE_LINK_MODE_PARAMS(100000, LR_ER_FR, 1, Full),
+ __DEFINE_LINK_MODE_PARAMS(100000, DR, 1, Full),
+ __DEFINE_LINK_MODE_PARAMS(100000, CR, 1, Full),
+ __DEFINE_LINK_MODE_PARAMS(200000, KR2, 2, Full),
+ __DEFINE_LINK_MODE_PARAMS(200000, SR2, 2, Full),
+ __DEFINE_LINK_MODE_PARAMS(200000, LR2_ER2_FR2, 2, Full),
+ __DEFINE_LINK_MODE_PARAMS(200000, DR2, 2, Full),
+ __DEFINE_LINK_MODE_PARAMS(200000, CR2, 2, Full),
+ __DEFINE_LINK_MODE_PARAMS(400000, KR4, 4, Full),
+ __DEFINE_LINK_MODE_PARAMS(400000, SR4, 4, Full),
+ __DEFINE_LINK_MODE_PARAMS(400000, LR4_ER4_FR4, 4, Full),
+ __DEFINE_LINK_MODE_PARAMS(400000, DR4, 4, Full),
+ __DEFINE_LINK_MODE_PARAMS(400000, CR4, 4, Full),
+ __DEFINE_LINK_MODE_PARAMS(100, FX, 1, Half),
+ __DEFINE_LINK_MODE_PARAMS(100, FX, 1, Full),
};
const struct nla_policy ethnl_linkmodes_set_policy[] = {
@@ -274,16 +277,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 +306,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,10 +332,10 @@ static bool ethnl_validate_master_slave_cfg(u8 cfg)
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;
+ bool req_speed, req_lanes, req_duplex;
const struct nlattr *master_slave_cfg;
int ret;
@@ -353,10 +358,39 @@ 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);
+
+ if (req_lanes) {
+ u32 lanes_cfg = nla_get_u32(tb[ETHTOOL_A_LINKMODES_LANES]);
+
+ if (!is_power_of_2(lanes_cfg)) {
+ NL_SET_ERR_MSG_ATTR(info->extack,
+ tb[ETHTOOL_A_LINKMODES_LANES],
+ "lanes value is invalid");
+ return -EINVAL;
+ }
+
+ /* 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,
+ tb[ETHTOOL_A_LINKMODES_LANES],
+ "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 UNKNOWN.
+ */
+ ksettings->lanes = ETHTOOL_LANES_UNKNOWN;
+ }
+
ret = ethnl_update_bitset(ksettings->link_modes.advertising,
__ETHTOOL_LINK_MODE_MASK_NBITS,
tb[ETHTOOL_A_LINKMODES_OURS], link_mode_names,
@@ -365,13 +399,15 @@ 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, tb[ETHTOOL_A_LINKMODES_LANES],
+ 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;
@@ -409,7 +445,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] 31+ messages in thread
* [PATCH net-next v3 2/7] ethtool: Get link mode in use instead of speed and duplex parameters
2021-01-20 9:37 [PATCH net-next v3 0/7] Support setting lanes via ethtool Danielle Ratson
2021-01-20 9:37 ` [PATCH net-next v3 1/7] ethtool: Extend link modes settings uAPI with lanes Danielle Ratson
@ 2021-01-20 9:37 ` Danielle Ratson
2021-01-20 23:39 ` Edwin Peer
2021-01-20 9:37 ` [PATCH net-next v3 3/7] ethtool: Expose the number of lanes in use Danielle Ratson
` (4 subsequent siblings)
6 siblings, 1 reply; 31+ messages in thread
From: Danielle Ratson @ 2021-01-20 9:37 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 | 114 ++++++++++++++++++++++++++++++++++++
net/ethtool/common.h | 7 +++
net/ethtool/ioctl.c | 18 +++++-
net/ethtool/linkmodes.c | 124 +---------------------------------------
5 files changed, 141 insertions(+), 123 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 24036e3055a1..399eee4a2051 100644
--- a/net/ethtool/common.c
+++ b/net/ethtool/common.c
@@ -197,6 +197,120 @@ const char link_mode_names[][ETH_GSTRING_LEN] = {
};
static_assert(ARRAY_SIZE(link_mode_names) == __ETHTOOL_LINK_MODE_MASK_NBITS);
+#define __DEFINE_LINK_MODE_PARAMS(_speed, _type, _lanes, _duplex) \
+ [ETHTOOL_LINK_MODE(_speed, _type, _duplex)] = { \
+ .speed = SPEED_ ## _speed, \
+ .lanes = _lanes, \
+ .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 = ETHTOOL_LANES_UNKNOWN, \
+ .duplex = DUPLEX_UNKNOWN, \
+ }
+
+const struct link_mode_info link_mode_params[] = {
+ __DEFINE_LINK_MODE_PARAMS(10, T, 1, Half),
+ __DEFINE_LINK_MODE_PARAMS(10, T, 1, Full),
+ __DEFINE_LINK_MODE_PARAMS(100, T, 1, Half),
+ __DEFINE_LINK_MODE_PARAMS(100, T, 1, Full),
+ __DEFINE_LINK_MODE_PARAMS(1000, T, 1, Half),
+ __DEFINE_LINK_MODE_PARAMS(1000, T, 1, 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, 1, Full),
+ __DEFINE_SPECIAL_MODE_PARAMS(Pause),
+ __DEFINE_SPECIAL_MODE_PARAMS(Asym_Pause),
+ __DEFINE_LINK_MODE_PARAMS(2500, X, 1, Full),
+ __DEFINE_SPECIAL_MODE_PARAMS(Backplane),
+ __DEFINE_LINK_MODE_PARAMS(1000, KX, 1, Full),
+ __DEFINE_LINK_MODE_PARAMS(10000, KX4, 4, Full),
+ __DEFINE_LINK_MODE_PARAMS(10000, KR, 1, Full),
+ [ETHTOOL_LINK_MODE_10000baseR_FEC_BIT] = {
+ .speed = SPEED_10000,
+ .duplex = DUPLEX_FULL,
+ },
+ __DEFINE_LINK_MODE_PARAMS(20000, MLD2, 2, Full),
+ __DEFINE_LINK_MODE_PARAMS(20000, KR2, 2, Full),
+ __DEFINE_LINK_MODE_PARAMS(40000, KR4, 4, Full),
+ __DEFINE_LINK_MODE_PARAMS(40000, CR4, 4, Full),
+ __DEFINE_LINK_MODE_PARAMS(40000, SR4, 4, Full),
+ __DEFINE_LINK_MODE_PARAMS(40000, LR4, 4, Full),
+ __DEFINE_LINK_MODE_PARAMS(56000, KR4, 4, Full),
+ __DEFINE_LINK_MODE_PARAMS(56000, CR4, 4, Full),
+ __DEFINE_LINK_MODE_PARAMS(56000, SR4, 4, Full),
+ __DEFINE_LINK_MODE_PARAMS(56000, LR4, 4, Full),
+ __DEFINE_LINK_MODE_PARAMS(25000, CR, 1, Full),
+ __DEFINE_LINK_MODE_PARAMS(25000, KR, 1, Full),
+ __DEFINE_LINK_MODE_PARAMS(25000, SR, 1, Full),
+ __DEFINE_LINK_MODE_PARAMS(50000, CR2, 2, Full),
+ __DEFINE_LINK_MODE_PARAMS(50000, KR2, 2, Full),
+ __DEFINE_LINK_MODE_PARAMS(100000, KR4, 4, Full),
+ __DEFINE_LINK_MODE_PARAMS(100000, SR4, 4, Full),
+ __DEFINE_LINK_MODE_PARAMS(100000, CR4, 4, Full),
+ __DEFINE_LINK_MODE_PARAMS(100000, LR4_ER4, 4, Full),
+ __DEFINE_LINK_MODE_PARAMS(50000, SR2, 2, Full),
+ __DEFINE_LINK_MODE_PARAMS(1000, X, 1, Full),
+ __DEFINE_LINK_MODE_PARAMS(10000, CR, 1, Full),
+ __DEFINE_LINK_MODE_PARAMS(10000, SR, 1, Full),
+ __DEFINE_LINK_MODE_PARAMS(10000, LR, 1, Full),
+ __DEFINE_LINK_MODE_PARAMS(10000, LRM, 1, Full),
+ __DEFINE_LINK_MODE_PARAMS(10000, ER, 1, Full),
+ __DEFINE_LINK_MODE_PARAMS(2500, T, 1, Full),
+ __DEFINE_LINK_MODE_PARAMS(5000, T, 1, 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, 1, Full),
+ __DEFINE_LINK_MODE_PARAMS(50000, SR, 1, Full),
+ __DEFINE_LINK_MODE_PARAMS(50000, CR, 1, Full),
+ __DEFINE_LINK_MODE_PARAMS(50000, LR_ER_FR, 1, Full),
+ __DEFINE_LINK_MODE_PARAMS(50000, DR, 1, Full),
+ __DEFINE_LINK_MODE_PARAMS(100000, KR2, 2, Full),
+ __DEFINE_LINK_MODE_PARAMS(100000, SR2, 2, Full),
+ __DEFINE_LINK_MODE_PARAMS(100000, CR2, 2, Full),
+ __DEFINE_LINK_MODE_PARAMS(100000, LR2_ER2_FR2, 2, Full),
+ __DEFINE_LINK_MODE_PARAMS(100000, DR2, 2, Full),
+ __DEFINE_LINK_MODE_PARAMS(200000, KR4, 4, Full),
+ __DEFINE_LINK_MODE_PARAMS(200000, SR4, 4, Full),
+ __DEFINE_LINK_MODE_PARAMS(200000, LR4_ER4_FR4, 4, Full),
+ __DEFINE_LINK_MODE_PARAMS(200000, DR4, 4, Full),
+ __DEFINE_LINK_MODE_PARAMS(200000, CR4, 4, Full),
+ __DEFINE_LINK_MODE_PARAMS(100, T1, 1, Full),
+ __DEFINE_LINK_MODE_PARAMS(1000, T1, 1, Full),
+ __DEFINE_LINK_MODE_PARAMS(400000, KR8, 8, Full),
+ __DEFINE_LINK_MODE_PARAMS(400000, SR8, 8, Full),
+ __DEFINE_LINK_MODE_PARAMS(400000, LR8_ER8_FR8, 8, Full),
+ __DEFINE_LINK_MODE_PARAMS(400000, DR8, 8, Full),
+ __DEFINE_LINK_MODE_PARAMS(400000, CR8, 8, Full),
+ __DEFINE_SPECIAL_MODE_PARAMS(FEC_LLRS),
+ __DEFINE_LINK_MODE_PARAMS(100000, KR, 1, Full),
+ __DEFINE_LINK_MODE_PARAMS(100000, SR, 1, Full),
+ __DEFINE_LINK_MODE_PARAMS(100000, LR_ER_FR, 1, Full),
+ __DEFINE_LINK_MODE_PARAMS(100000, DR, 1, Full),
+ __DEFINE_LINK_MODE_PARAMS(100000, CR, 1, Full),
+ __DEFINE_LINK_MODE_PARAMS(200000, KR2, 2, Full),
+ __DEFINE_LINK_MODE_PARAMS(200000, SR2, 2, Full),
+ __DEFINE_LINK_MODE_PARAMS(200000, LR2_ER2_FR2, 2, Full),
+ __DEFINE_LINK_MODE_PARAMS(200000, DR2, 2, Full),
+ __DEFINE_LINK_MODE_PARAMS(200000, CR2, 2, Full),
+ __DEFINE_LINK_MODE_PARAMS(400000, KR4, 4, Full),
+ __DEFINE_LINK_MODE_PARAMS(400000, SR4, 4, Full),
+ __DEFINE_LINK_MODE_PARAMS(400000, LR4_ER4_FR4, 4, Full),
+ __DEFINE_LINK_MODE_PARAMS(400000, DR4, 4, Full),
+ __DEFINE_LINK_MODE_PARAMS(400000, CR4, 4, Full),
+ __DEFINE_LINK_MODE_PARAMS(100, FX, 1, Half),
+ __DEFINE_LINK_MODE_PARAMS(100, FX, 1, 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..625c8472700c 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;
+ u32 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 fb7d73250864..caf9111c5270 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,125 +152,6 @@ const struct ethnl_request_ops ethnl_linkmodes_request_ops = {
/* LINKMODES_SET */
-struct link_mode_info {
- int speed;
- u32 lanes;
- u8 duplex;
-};
-
-#define __DEFINE_LINK_MODE_PARAMS(_speed, _type, _lanes, _duplex) \
- [ETHTOOL_LINK_MODE(_speed, _type, _duplex)] = { \
- .speed = SPEED_ ## _speed, \
- .lanes = _lanes, \
- .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 = ETHTOOL_LANES_UNKNOWN, \
- .duplex = DUPLEX_UNKNOWN, \
- }
-
-static const struct link_mode_info link_mode_params[] = {
- __DEFINE_LINK_MODE_PARAMS(10, T, 1, Half),
- __DEFINE_LINK_MODE_PARAMS(10, T, 1, Full),
- __DEFINE_LINK_MODE_PARAMS(100, T, 1, Half),
- __DEFINE_LINK_MODE_PARAMS(100, T, 1, Full),
- __DEFINE_LINK_MODE_PARAMS(1000, T, 1, Half),
- __DEFINE_LINK_MODE_PARAMS(1000, T, 1, 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, 1, Full),
- __DEFINE_SPECIAL_MODE_PARAMS(Pause),
- __DEFINE_SPECIAL_MODE_PARAMS(Asym_Pause),
- __DEFINE_LINK_MODE_PARAMS(2500, X, 1, Full),
- __DEFINE_SPECIAL_MODE_PARAMS(Backplane),
- __DEFINE_LINK_MODE_PARAMS(1000, KX, 1, Full),
- __DEFINE_LINK_MODE_PARAMS(10000, KX4, 4, Full),
- __DEFINE_LINK_MODE_PARAMS(10000, KR, 1, Full),
- [ETHTOOL_LINK_MODE_10000baseR_FEC_BIT] = {
- .speed = SPEED_10000,
- .duplex = DUPLEX_FULL,
- },
- __DEFINE_LINK_MODE_PARAMS(20000, MLD2, 2, Full),
- __DEFINE_LINK_MODE_PARAMS(20000, KR2, 2, Full),
- __DEFINE_LINK_MODE_PARAMS(40000, KR4, 4, Full),
- __DEFINE_LINK_MODE_PARAMS(40000, CR4, 4, Full),
- __DEFINE_LINK_MODE_PARAMS(40000, SR4, 4, Full),
- __DEFINE_LINK_MODE_PARAMS(40000, LR4, 4, Full),
- __DEFINE_LINK_MODE_PARAMS(56000, KR4, 4, Full),
- __DEFINE_LINK_MODE_PARAMS(56000, CR4, 4, Full),
- __DEFINE_LINK_MODE_PARAMS(56000, SR4, 4, Full),
- __DEFINE_LINK_MODE_PARAMS(56000, LR4, 4, Full),
- __DEFINE_LINK_MODE_PARAMS(25000, CR, 1, Full),
- __DEFINE_LINK_MODE_PARAMS(25000, KR, 1, Full),
- __DEFINE_LINK_MODE_PARAMS(25000, SR, 1, Full),
- __DEFINE_LINK_MODE_PARAMS(50000, CR2, 2, Full),
- __DEFINE_LINK_MODE_PARAMS(50000, KR2, 2, Full),
- __DEFINE_LINK_MODE_PARAMS(100000, KR4, 4, Full),
- __DEFINE_LINK_MODE_PARAMS(100000, SR4, 4, Full),
- __DEFINE_LINK_MODE_PARAMS(100000, CR4, 4, Full),
- __DEFINE_LINK_MODE_PARAMS(100000, LR4_ER4, 4, Full),
- __DEFINE_LINK_MODE_PARAMS(50000, SR2, 2, Full),
- __DEFINE_LINK_MODE_PARAMS(1000, X, 1, Full),
- __DEFINE_LINK_MODE_PARAMS(10000, CR, 1, Full),
- __DEFINE_LINK_MODE_PARAMS(10000, SR, 1, Full),
- __DEFINE_LINK_MODE_PARAMS(10000, LR, 1, Full),
- __DEFINE_LINK_MODE_PARAMS(10000, LRM, 1, Full),
- __DEFINE_LINK_MODE_PARAMS(10000, ER, 1, Full),
- __DEFINE_LINK_MODE_PARAMS(2500, T, 1, Full),
- __DEFINE_LINK_MODE_PARAMS(5000, T, 1, 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, 1, Full),
- __DEFINE_LINK_MODE_PARAMS(50000, SR, 1, Full),
- __DEFINE_LINK_MODE_PARAMS(50000, CR, 1, Full),
- __DEFINE_LINK_MODE_PARAMS(50000, LR_ER_FR, 1, Full),
- __DEFINE_LINK_MODE_PARAMS(50000, DR, 1, Full),
- __DEFINE_LINK_MODE_PARAMS(100000, KR2, 2, Full),
- __DEFINE_LINK_MODE_PARAMS(100000, SR2, 2, Full),
- __DEFINE_LINK_MODE_PARAMS(100000, CR2, 2, Full),
- __DEFINE_LINK_MODE_PARAMS(100000, LR2_ER2_FR2, 2, Full),
- __DEFINE_LINK_MODE_PARAMS(100000, DR2, 2, Full),
- __DEFINE_LINK_MODE_PARAMS(200000, KR4, 4, Full),
- __DEFINE_LINK_MODE_PARAMS(200000, SR4, 4, Full),
- __DEFINE_LINK_MODE_PARAMS(200000, LR4_ER4_FR4, 4, Full),
- __DEFINE_LINK_MODE_PARAMS(200000, DR4, 4, Full),
- __DEFINE_LINK_MODE_PARAMS(200000, CR4, 4, Full),
- __DEFINE_LINK_MODE_PARAMS(100, T1, 1, Full),
- __DEFINE_LINK_MODE_PARAMS(1000, T1, 1, Full),
- __DEFINE_LINK_MODE_PARAMS(400000, KR8, 8, Full),
- __DEFINE_LINK_MODE_PARAMS(400000, SR8, 8, Full),
- __DEFINE_LINK_MODE_PARAMS(400000, LR8_ER8_FR8, 8, Full),
- __DEFINE_LINK_MODE_PARAMS(400000, DR8, 8, Full),
- __DEFINE_LINK_MODE_PARAMS(400000, CR8, 8, Full),
- __DEFINE_SPECIAL_MODE_PARAMS(FEC_LLRS),
- __DEFINE_LINK_MODE_PARAMS(100000, KR, 1, Full),
- __DEFINE_LINK_MODE_PARAMS(100000, SR, 1, Full),
- __DEFINE_LINK_MODE_PARAMS(100000, LR_ER_FR, 1, Full),
- __DEFINE_LINK_MODE_PARAMS(100000, DR, 1, Full),
- __DEFINE_LINK_MODE_PARAMS(100000, CR, 1, Full),
- __DEFINE_LINK_MODE_PARAMS(200000, KR2, 2, Full),
- __DEFINE_LINK_MODE_PARAMS(200000, SR2, 2, Full),
- __DEFINE_LINK_MODE_PARAMS(200000, LR2_ER2_FR2, 2, Full),
- __DEFINE_LINK_MODE_PARAMS(200000, DR2, 2, Full),
- __DEFINE_LINK_MODE_PARAMS(200000, CR2, 2, Full),
- __DEFINE_LINK_MODE_PARAMS(400000, KR4, 4, Full),
- __DEFINE_LINK_MODE_PARAMS(400000, SR4, 4, Full),
- __DEFINE_LINK_MODE_PARAMS(400000, LR4_ER4_FR4, 4, Full),
- __DEFINE_LINK_MODE_PARAMS(400000, DR4, 4, Full),
- __DEFINE_LINK_MODE_PARAMS(400000, CR4, 4, Full),
- __DEFINE_LINK_MODE_PARAMS(100, FX, 1, Half),
- __DEFINE_LINK_MODE_PARAMS(100, FX, 1, Full),
-};
-
const struct nla_policy ethnl_linkmodes_set_policy[] = {
[ETHTOOL_A_LINKMODES_HEADER] =
NLA_POLICY_NESTED(ethnl_header_policy),
@@ -294,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] 31+ messages in thread
* [PATCH net-next v3 3/7] ethtool: Expose the number of lanes in use
2021-01-20 9:37 [PATCH net-next v3 0/7] Support setting lanes via ethtool Danielle Ratson
2021-01-20 9:37 ` [PATCH net-next v3 1/7] ethtool: Extend link modes settings uAPI with lanes Danielle Ratson
2021-01-20 9:37 ` [PATCH net-next v3 2/7] ethtool: Get link mode in use instead of speed and duplex parameters Danielle Ratson
@ 2021-01-20 9:37 ` Danielle Ratson
2021-01-20 9:37 ` [PATCH net-next v3 4/7] mlxsw: ethtool: Remove max lanes filtering Danielle Ratson
` (3 subsequent siblings)
6 siblings, 0 replies; 31+ messages in thread
From: Danielle Ratson @ 2021-01-20 9:37 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 | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/net/ethtool/linkmodes.c b/net/ethtool/linkmodes.c
index caf9111c5270..0c18f8eda63d 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 = ETHTOOL_LANES_UNKNOWN;
+
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,
@@ -122,6 +126,7 @@ static int linkmodes_fill_reply(struct sk_buff *skb,
}
if (nla_put_u32(skb, ETHTOOL_A_LINKMODES_SPEED, lsettings->speed) ||
+ nla_put_u32(skb, ETHTOOL_A_LINKMODES_LANES, ksettings->lanes) ||
nla_put_u8(skb, ETHTOOL_A_LINKMODES_DUPLEX, lsettings->duplex))
return -EMSGSIZE;
--
2.26.2
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH net-next v3 4/7] mlxsw: ethtool: Remove max lanes filtering
2021-01-20 9:37 [PATCH net-next v3 0/7] Support setting lanes via ethtool Danielle Ratson
` (2 preceding siblings ...)
2021-01-20 9:37 ` [PATCH net-next v3 3/7] ethtool: Expose the number of lanes in use Danielle Ratson
@ 2021-01-20 9:37 ` Danielle Ratson
2021-01-20 9:37 ` [PATCH net-next v3 5/7] mlxsw: ethtool: Add support for setting lanes when autoneg is off Danielle Ratson
` (2 subsequent siblings)
6 siblings, 0 replies; 31+ messages in thread
From: Danielle Ratson @ 2021-01-20 9:37 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] 31+ messages in thread
* [PATCH net-next v3 5/7] mlxsw: ethtool: Add support for setting lanes when autoneg is off
2021-01-20 9:37 [PATCH net-next v3 0/7] Support setting lanes via ethtool Danielle Ratson
` (3 preceding siblings ...)
2021-01-20 9:37 ` [PATCH net-next v3 4/7] mlxsw: ethtool: Remove max lanes filtering Danielle Ratson
@ 2021-01-20 9:37 ` Danielle Ratson
2021-01-20 9:37 ` [PATCH net-next v3 6/7] mlxsw: ethtool: Pass link mode in use to ethtool Danielle Ratson
2021-01-20 9:37 ` [PATCH net-next v3 7/7] net: selftests: Add lanes setting test Danielle Ratson
6 siblings, 0 replies; 31+ messages in thread
From: Danielle Ratson @ 2021-01-20 9:37 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..a3c006fb0fdf 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 == ETHTOOL_LANES_UNKNOWN) {
+ /* 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] 31+ messages in thread
* [PATCH net-next v3 6/7] mlxsw: ethtool: Pass link mode in use to ethtool
2021-01-20 9:37 [PATCH net-next v3 0/7] Support setting lanes via ethtool Danielle Ratson
` (4 preceding siblings ...)
2021-01-20 9:37 ` [PATCH net-next v3 5/7] mlxsw: ethtool: Add support for setting lanes when autoneg is off Danielle Ratson
@ 2021-01-20 9:37 ` Danielle Ratson
2021-01-20 9:37 ` [PATCH net-next v3 7/7] net: selftests: Add lanes setting test Danielle Ratson
6 siblings, 0 replies; 31+ messages in thread
From: Danielle Ratson @ 2021-01-20 9:37 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 a3c006fb0fdf..461918f8310c 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] 31+ messages in thread
* [PATCH net-next v3 7/7] net: selftests: Add lanes setting test
2021-01-20 9:37 [PATCH net-next v3 0/7] Support setting lanes via ethtool Danielle Ratson
` (5 preceding siblings ...)
2021-01-20 9:37 ` [PATCH net-next v3 6/7] mlxsw: ethtool: Pass link mode in use to ethtool Danielle Ratson
@ 2021-01-20 9:37 ` Danielle Ratson
6 siblings, 0 replies; 31+ messages in thread
From: Danielle Ratson @ 2021-01-20 9:37 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:
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 | 188 ++++++++++++++++++
.../selftests/net/forwarding/ethtool_lib.sh | 34 ++++
tools/testing/selftests/net/forwarding/lib.sh | 28 +++
3 files changed, 250 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..faa0bddc3e63
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/mlxsw/ethtool_lanes.sh
@@ -0,0 +1,188 @@
+#!/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 chosen_lanes=$(ethtool $swp1 | grep 'Lanes:')
+ chosen_lanes=${chosen_lanes#*"Lanes: "}
+ if [[ $chosen_lanes == "Unknown!" ]]; 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] 31+ messages in thread
* Re: [PATCH net-next v3 1/7] ethtool: Extend link modes settings uAPI with lanes
2021-01-20 9:37 ` [PATCH net-next v3 1/7] ethtool: Extend link modes settings uAPI with lanes Danielle Ratson
@ 2021-01-20 22:35 ` Edwin Peer
2021-02-02 18:08 ` Danielle Ratson
2021-01-22 3:44 ` Jakub Kicinski
1 sibling, 1 reply; 31+ messages in thread
From: Edwin Peer @ 2021-01-20 22:35 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: 11375 bytes --]
On Wed, Jan 20, 2021 at 3:21 AM Danielle Ratson <danieller@nvidia.com> wrote:
> -#define __DEFINE_LINK_MODE_PARAMS(_speed, _type, _duplex) \
> +#define __DEFINE_LINK_MODE_PARAMS(_speed, _type, _lanes, _duplex) \
> [ETHTOOL_LINK_MODE(_speed, _type, _duplex)] = { \
> .speed = SPEED_ ## _speed, \
> + .lanes = _lanes, \
> .duplex = __DUPLEX_ ## _duplex \
> }
What about:
#define __DECLARE_LINK_MODE_LANES(_type, _lanes) \
static const u32 __LINK_MODE_LANES_ ## _type = _lanes;
#define __DECLARE_LINK_MODE_LANES_ALL(_type) \
__DECLARE_LINK_MODE_LANES(_type, 1) \
__DECLARE_LINK_MODE_LANES(_type ## 2, 2) \
__DECLARE_LINK_MODE_LANES(_type ## 4, 4) \
__DECLARE_LINK_MODE_LANES(_type ## 8, 8)
__DECLARE_LINK_MODE_LANES_ALL(CR)
__DECLARE_LINK_MODE_LANES_ALL(DR)
__DECLARE_LINK_MODE_LANES_ALL(ER)
__DECLARE_LINK_MODE_LANES_ALL(KR)
__DECLARE_LINK_MODE_LANES(KX, 1)
__DECLARE_LINK_MODE_LANES(KX4, 4)
__DECLARE_LINK_MODE_LANES_ALL(LR)
__DECLARE_LINK_MODE_LANES(LR_ER_FR, 1)
__DECLARE_LINK_MODE_LANES(LR2_ER2_FR2, 2)
__DECLARE_LINK_MODE_LANES(LR4_ER4_FR4, 4)
__DECLARE_LINK_MODE_LANES(LR8_ER8_FR8, 8)
__DECLARE_LINK_MODE_LANES(LRM, 1)
__DECLARE_LINK_MODE_LANES(MLD2, 2);
__DECLARE_LINK_MODE_LANES_ALL(SR);
__DECLARE_LINK_MODE_LANES(T, 1)
__DECLARE_LINK_MODE_LANES(X, 1)
#define __DEFINE_LINK_MODE_PARAMS(_speed, _type, _duplex)
[ETHTOOL_LINK_MODE(_speed, _type, _duplex)] = { \
.speed = SPEED_ ## _speed, \
.lanes = __LINK_MODE_LANES ## _type, \
instead of specifying lanes for each link mode defined below?
Regards,
Edwin Peer
> 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_LINK_MODE_PARAMS(10, T, 1, Half),
> + __DEFINE_LINK_MODE_PARAMS(10, T, 1, Full),
> + __DEFINE_LINK_MODE_PARAMS(100, T, 1, Half),
> + __DEFINE_LINK_MODE_PARAMS(100, T, 1, Full),
> + __DEFINE_LINK_MODE_PARAMS(1000, T, 1, Half),
> + __DEFINE_LINK_MODE_PARAMS(1000, T, 1, 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_LINK_MODE_PARAMS(10000, T, 1, Full),
> __DEFINE_SPECIAL_MODE_PARAMS(Pause),
> __DEFINE_SPECIAL_MODE_PARAMS(Asym_Pause),
> - __DEFINE_LINK_MODE_PARAMS(2500, X, Full),
> + __DEFINE_LINK_MODE_PARAMS(2500, X, 1, 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),
> + __DEFINE_LINK_MODE_PARAMS(1000, KX, 1, Full),
> + __DEFINE_LINK_MODE_PARAMS(10000, KX4, 4, Full),
> + __DEFINE_LINK_MODE_PARAMS(10000, KR, 1, 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_LINK_MODE_PARAMS(20000, MLD2, 2, Full),
> + __DEFINE_LINK_MODE_PARAMS(20000, KR2, 2, Full),
> + __DEFINE_LINK_MODE_PARAMS(40000, KR4, 4, Full),
> + __DEFINE_LINK_MODE_PARAMS(40000, CR4, 4, Full),
> + __DEFINE_LINK_MODE_PARAMS(40000, SR4, 4, Full),
> + __DEFINE_LINK_MODE_PARAMS(40000, LR4, 4, Full),
> + __DEFINE_LINK_MODE_PARAMS(56000, KR4, 4, Full),
> + __DEFINE_LINK_MODE_PARAMS(56000, CR4, 4, Full),
> + __DEFINE_LINK_MODE_PARAMS(56000, SR4, 4, Full),
> + __DEFINE_LINK_MODE_PARAMS(56000, LR4, 4, Full),
> + __DEFINE_LINK_MODE_PARAMS(25000, CR, 1, Full),
> + __DEFINE_LINK_MODE_PARAMS(25000, KR, 1, Full),
> + __DEFINE_LINK_MODE_PARAMS(25000, SR, 1, Full),
> + __DEFINE_LINK_MODE_PARAMS(50000, CR2, 2, Full),
> + __DEFINE_LINK_MODE_PARAMS(50000, KR2, 2, Full),
> + __DEFINE_LINK_MODE_PARAMS(100000, KR4, 4, Full),
> + __DEFINE_LINK_MODE_PARAMS(100000, SR4, 4, Full),
> + __DEFINE_LINK_MODE_PARAMS(100000, CR4, 4, Full),
> + __DEFINE_LINK_MODE_PARAMS(100000, LR4_ER4, 4, Full),
> + __DEFINE_LINK_MODE_PARAMS(50000, SR2, 2, Full),
> + __DEFINE_LINK_MODE_PARAMS(1000, X, 1, Full),
> + __DEFINE_LINK_MODE_PARAMS(10000, CR, 1, Full),
> + __DEFINE_LINK_MODE_PARAMS(10000, SR, 1, Full),
> + __DEFINE_LINK_MODE_PARAMS(10000, LR, 1, Full),
> + __DEFINE_LINK_MODE_PARAMS(10000, LRM, 1, Full),
> + __DEFINE_LINK_MODE_PARAMS(10000, ER, 1, Full),
> + __DEFINE_LINK_MODE_PARAMS(2500, T, 1, Full),
> + __DEFINE_LINK_MODE_PARAMS(5000, T, 1, 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_LINK_MODE_PARAMS(50000, KR, 1, Full),
> + __DEFINE_LINK_MODE_PARAMS(50000, SR, 1, Full),
> + __DEFINE_LINK_MODE_PARAMS(50000, CR, 1, Full),
> + __DEFINE_LINK_MODE_PARAMS(50000, LR_ER_FR, 1, Full),
> + __DEFINE_LINK_MODE_PARAMS(50000, DR, 1, Full),
> + __DEFINE_LINK_MODE_PARAMS(100000, KR2, 2, Full),
> + __DEFINE_LINK_MODE_PARAMS(100000, SR2, 2, Full),
> + __DEFINE_LINK_MODE_PARAMS(100000, CR2, 2, Full),
> + __DEFINE_LINK_MODE_PARAMS(100000, LR2_ER2_FR2, 2, Full),
> + __DEFINE_LINK_MODE_PARAMS(100000, DR2, 2, Full),
> + __DEFINE_LINK_MODE_PARAMS(200000, KR4, 4, Full),
> + __DEFINE_LINK_MODE_PARAMS(200000, SR4, 4, Full),
> + __DEFINE_LINK_MODE_PARAMS(200000, LR4_ER4_FR4, 4, Full),
> + __DEFINE_LINK_MODE_PARAMS(200000, DR4, 4, Full),
> + __DEFINE_LINK_MODE_PARAMS(200000, CR4, 4, Full),
> + __DEFINE_LINK_MODE_PARAMS(100, T1, 1, Full),
> + __DEFINE_LINK_MODE_PARAMS(1000, T1, 1, Full),
> + __DEFINE_LINK_MODE_PARAMS(400000, KR8, 8, Full),
> + __DEFINE_LINK_MODE_PARAMS(400000, SR8, 8, Full),
> + __DEFINE_LINK_MODE_PARAMS(400000, LR8_ER8_FR8, 8, Full),
> + __DEFINE_LINK_MODE_PARAMS(400000, DR8, 8, Full),
> + __DEFINE_LINK_MODE_PARAMS(400000, CR8, 8, 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),
> + __DEFINE_LINK_MODE_PARAMS(100000, KR, 1, Full),
> + __DEFINE_LINK_MODE_PARAMS(100000, SR, 1, Full),
> + __DEFINE_LINK_MODE_PARAMS(100000, LR_ER_FR, 1, Full),
> + __DEFINE_LINK_MODE_PARAMS(100000, DR, 1, Full),
> + __DEFINE_LINK_MODE_PARAMS(100000, CR, 1, Full),
> + __DEFINE_LINK_MODE_PARAMS(200000, KR2, 2, Full),
> + __DEFINE_LINK_MODE_PARAMS(200000, SR2, 2, Full),
> + __DEFINE_LINK_MODE_PARAMS(200000, LR2_ER2_FR2, 2, Full),
> + __DEFINE_LINK_MODE_PARAMS(200000, DR2, 2, Full),
> + __DEFINE_LINK_MODE_PARAMS(200000, CR2, 2, Full),
> + __DEFINE_LINK_MODE_PARAMS(400000, KR4, 4, Full),
> + __DEFINE_LINK_MODE_PARAMS(400000, SR4, 4, Full),
> + __DEFINE_LINK_MODE_PARAMS(400000, LR4_ER4_FR4, 4, Full),
> + __DEFINE_LINK_MODE_PARAMS(400000, DR4, 4, Full),
> + __DEFINE_LINK_MODE_PARAMS(400000, CR4, 4, Full),
> + __DEFINE_LINK_MODE_PARAMS(100, FX, 1, Half),
> + __DEFINE_LINK_MODE_PARAMS(100, FX, 1, Full),
> };
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4160 bytes --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH net-next v3 2/7] ethtool: Get link mode in use instead of speed and duplex parameters
2021-01-20 9:37 ` [PATCH net-next v3 2/7] ethtool: Get link mode in use instead of speed and duplex parameters Danielle Ratson
@ 2021-01-20 23:39 ` Edwin Peer
2021-01-24 8:36 ` Danielle Ratson
0 siblings, 1 reply; 31+ messages in thread
From: Edwin Peer @ 2021-01-20 23:39 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: 975 bytes --]
On Wed, Jan 20, 2021 at 3:21 AM Danielle Ratson <danieller@nvidia.com> wrote:
> + 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;
> + }
Why isn't this also handled using a capability bit as is done for
lanes? Is link_mode read-only? Should it / will it always be? If not,
can drivers also derive the other fields if asked to set link_mode?
That would be easy enough. Why don't we simply allow user space to set
link mode directly too (in addition to being able to constrain lanes
for autoneg or forced speeds)?
Regards,
Edwin Peer
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4160 bytes --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH net-next v3 1/7] ethtool: Extend link modes settings uAPI with lanes
2021-01-20 9:37 ` [PATCH net-next v3 1/7] ethtool: Extend link modes settings uAPI with lanes Danielle Ratson
2021-01-20 22:35 ` Edwin Peer
@ 2021-01-22 3:44 ` Jakub Kicinski
2021-01-25 15:53 ` Danielle Ratson
1 sibling, 1 reply; 31+ messages in thread
From: Jakub Kicinski @ 2021-01-22 3:44 UTC (permalink / raw)
To: Danielle Ratson
Cc: netdev, davem, jiri, andrew, f.fainelli, mkubecek, mlxsw, idosch
On Wed, 20 Jan 2021 11:37:07 +0200 Danielle Ratson wrote:
> 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.
> Signed-off-by: Danielle Ratson <danieller@nvidia.com>
> diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
> index cde753bb2093..80edae2c24f7 100644
> --- a/include/uapi/linux/ethtool.h
> +++ b/include/uapi/linux/ethtool.h
> @@ -1738,6 +1738,8 @@ static inline int ethtool_validate_speed(__u32 speed)
> return speed <= INT_MAX || speed == (__u32)SPEED_UNKNOWN;
> }
>
> +#define ETHTOOL_LANES_UNKNOWN 0
I already complained about these unnecessary uAPI constants, did you
reply to that and I missed it?
Don't report the nlattr if it's unknown, we have netlink now, those
constants are from times when we returned structures and all fields
had to have a value.
> /* Duplex, half or full. */
> #define DUPLEX_HALF 0x00
> #define DUPLEX_FULL 0x01
> 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 c5bcb9abc8b9..fb7d73250864 100644
> --- a/net/ethtool/linkmodes.c
> +++ b/net/ethtool/linkmodes.c
> @@ -152,12 +152,14 @@ const struct ethnl_request_ops ethnl_linkmodes_request_ops = {
>
> struct link_mode_info {
> int speed;
> + u32 lanes;
This is not uapi, we can make it u8 now, save a few (hundred?) bytes
of memory and bump it to u16 later.
> u8 duplex;
> };
> @@ -353,10 +358,39 @@ 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);
> +
> + if (req_lanes) {
> + u32 lanes_cfg = nla_get_u32(tb[ETHTOOL_A_LINKMODES_LANES]);
req_lanes == tb[ETHTOOL_A_LINKMODES_LANES], right?
Please use req_lanes variable where possible.
> +
> + if (!is_power_of_2(lanes_cfg)) {
> + NL_SET_ERR_MSG_ATTR(info->extack,
> + tb[ETHTOOL_A_LINKMODES_LANES],
> + "lanes value is invalid");
> + return -EINVAL;
> + }
> +
> + /* 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,
> + tb[ETHTOOL_A_LINKMODES_LANES],
> + "lanes configuration not supported by device");
> + return -EOPNOTSUPP;
> + }
This validation does not depend on the current settings at all, it's
just input validation, it can be done before rtnl_lock is taken (in a
new function).
You can move ethnl_validate_master_slave_cfg() to that function as well
(as a cleanup before this patch).
> + } else if (!lsettings->autoneg) {
> + /* If autoneg is off and lanes parameter is not passed from user,
> + * set the lanes parameter to UNKNOWN.
> + */
> + ksettings->lanes = ETHTOOL_LANES_UNKNOWN;
> + }
> +
> ret = ethnl_update_bitset(ksettings->link_modes.advertising,
> __ETHTOOL_LINK_MODE_MASK_NBITS,
> tb[ETHTOOL_A_LINKMODES_OURS], link_mode_names,
^ permalink raw reply [flat|nested] 31+ messages in thread
* RE: [PATCH net-next v3 2/7] ethtool: Get link mode in use instead of speed and duplex parameters
2021-01-20 23:39 ` Edwin Peer
@ 2021-01-24 8:36 ` Danielle Ratson
2021-01-25 18:03 ` Edwin Peer
0 siblings, 1 reply; 31+ messages in thread
From: Danielle Ratson @ 2021-01-24 8:36 UTC (permalink / raw)
To: Edwin Peer
Cc: netdev, David S . Miller, Jakub Kicinski, Jiri Pirko,
Andrew Lunn, f.fainelli, Michal Kubecek, mlxsw, Ido Schimmel
> -----Original Message-----
> From: Edwin Peer <edwin.peer@broadcom.com>
> Sent: Thursday, January 21, 2021 1:39 AM
> To: Danielle Ratson <danieller@nvidia.com>
> Cc: netdev <netdev@vger.kernel.org>; David S . Miller <davem@davemloft.net>; Jakub Kicinski <kuba@kernel.org>; Jiri Pirko
> <jiri@nvidia.com>; Andrew Lunn <andrew@lunn.ch>; f.fainelli@gmail.com; Michal Kubecek <mkubecek@suse.cz>; mlxsw
> <mlxsw@nvidia.com>; Ido Schimmel <idosch@nvidia.com>
> Subject: Re: [PATCH net-next v3 2/7] ethtool: Get link mode in use instead of speed and duplex parameters
>
> On Wed, Jan 20, 2021 at 3:21 AM Danielle Ratson <danieller@nvidia.com> wrote:
>
> > + 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;
> > + }
>
> Why isn't this also handled using a capability bit as is done for
> lanes? Is link_mode read-only? Should it / will it always be? If not,
> can drivers also derive the other fields if asked to set link_mode?
The link_mode param is only for deriving all the speed, lanes and duplex params in ethtool instead of deriving in driver and then passing each individual, as Michal asked.
> That would be easy enough. Why don't we simply allow user space to set
> link mode directly too (in addition to being able to constrain lanes
> for autoneg or forced speeds)?
It is already possible to do using 'advertise' parameter.
>
> Regards,
> Edwin Peer
^ permalink raw reply [flat|nested] 31+ messages in thread
* RE: [PATCH net-next v3 1/7] ethtool: Extend link modes settings uAPI with lanes
2021-01-22 3:44 ` Jakub Kicinski
@ 2021-01-25 15:53 ` Danielle Ratson
2021-01-25 19:02 ` Jakub Kicinski
0 siblings, 1 reply; 31+ messages in thread
From: Danielle Ratson @ 2021-01-25 15:53 UTC (permalink / raw)
To: Jakub Kicinski
Cc: netdev, davem, Jiri Pirko, andrew, f.fainelli, mkubecek, mlxsw,
Ido Schimmel
> -----Original Message-----
> From: Jakub Kicinski <kuba@kernel.org>
> Sent: Friday, January 22, 2021 5:45 AM
> To: Danielle Ratson <danieller@nvidia.com>
> Cc: netdev@vger.kernel.org; davem@davemloft.net; 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 v3 1/7] ethtool: Extend link modes settings uAPI with lanes
>
> On Wed, 20 Jan 2021 11:37:07 +0200 Danielle Ratson wrote:
> > 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.
>
> > Signed-off-by: Danielle Ratson <danieller@nvidia.com>
>
> > diff --git a/include/uapi/linux/ethtool.h
> > b/include/uapi/linux/ethtool.h index cde753bb2093..80edae2c24f7 100644
> > --- a/include/uapi/linux/ethtool.h
> > +++ b/include/uapi/linux/ethtool.h
> > @@ -1738,6 +1738,8 @@ static inline int ethtool_validate_speed(__u32 speed)
> > return speed <= INT_MAX || speed == (__u32)SPEED_UNKNOWN; }
> >
> > +#define ETHTOOL_LANES_UNKNOWN 0
>
> I already complained about these unnecessary uAPI constants, did you reply to that and I missed it?
I guess I missed it, sorry, will fix.
>
> Don't report the nlattr if it's unknown, we have netlink now, those constants are from times when we returned structures and all
> fields had to have a value.
>
> > /* Duplex, half or full. */
> > #define DUPLEX_HALF 0x00
> > #define DUPLEX_FULL 0x01
> > 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
> > c5bcb9abc8b9..fb7d73250864 100644
> > --- a/net/ethtool/linkmodes.c
> > +++ b/net/ethtool/linkmodes.c
> > @@ -152,12 +152,14 @@ const struct ethnl_request_ops
> > ethnl_linkmodes_request_ops = {
> >
> > struct link_mode_info {
> > int speed;
> > + u32 lanes;
>
> This is not uapi, we can make it u8 now, save a few (hundred?) bytes of memory and bump it to u16 later.
>
> > u8 duplex;
> > };
>
> > @@ -353,10 +358,39 @@ 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);
> > +
> > + if (req_lanes) {
> > + u32 lanes_cfg = nla_get_u32(tb[ETHTOOL_A_LINKMODES_LANES]);
>
> req_lanes == tb[ETHTOOL_A_LINKMODES_LANES], right?
Yes, but req_lanes is a bool and doesn't fit to nla_get_u32. Do you want me to change the req_lanes type and name?
>
> Please use req_lanes variable where possible.
>
> > +
> > + if (!is_power_of_2(lanes_cfg)) {
> > + NL_SET_ERR_MSG_ATTR(info->extack,
> > + tb[ETHTOOL_A_LINKMODES_LANES],
> > + "lanes value is invalid");
> > + return -EINVAL;
> > + }
> > +
> > + /* 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,
> > + tb[ETHTOOL_A_LINKMODES_LANES],
> > + "lanes configuration not supported by device");
> > + return -EOPNOTSUPP;
> > + }
>
> This validation does not depend on the current settings at all, it's just input validation, it can be done before rtnl_lock is taken (in a
> new function).
>
> You can move ethnl_validate_master_slave_cfg() to that function as well (as a cleanup before this patch).
Do you mean to move the ethnl_validate_master_slave_cfg() if from that function? Doesn't it depend on the current settings, as opposed to the supported lanes param that you wanted me to move as well?
Not sure I understand the second part of the request...
>
> > + } else if (!lsettings->autoneg) {
> > + /* If autoneg is off and lanes parameter is not passed from user,
> > + * set the lanes parameter to UNKNOWN.
> > + */
> > + ksettings->lanes = ETHTOOL_LANES_UNKNOWN;
> > + }
> > +
> > ret = ethnl_update_bitset(ksettings->link_modes.advertising,
> > __ETHTOOL_LINK_MODE_MASK_NBITS,
> > tb[ETHTOOL_A_LINKMODES_OURS], link_mode_names,
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH net-next v3 2/7] ethtool: Get link mode in use instead of speed and duplex parameters
2021-01-24 8:36 ` Danielle Ratson
@ 2021-01-25 18:03 ` Edwin Peer
2021-01-26 17:06 ` Danielle Ratson
0 siblings, 1 reply; 31+ messages in thread
From: Edwin Peer @ 2021-01-25 18:03 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: 1238 bytes --]
On Sun, Jan 24, 2021 at 12:36 AM Danielle Ratson <danieller@nvidia.com> wrote:
> > Why isn't this also handled using a capability bit as is done for
> > lanes? Is link_mode read-only? Should it / will it always be? If not,
> > can drivers also derive the other fields if asked to set link_mode?
>
> The link_mode param is only for deriving all the speed, lanes and duplex params in ethtool instead of deriving in driver and then > passing each individual, as Michal asked.
I understand the benefit of deriving the dependent fields in core code
rather than in each driver, I just don't think this is necessarily
mutually exclusive with being able to force a particular link mode at
the driver API, making link_mode R/W (and even extend this interface
to user space). For a driver that works internally in terms of the
link_mode it's returning, this would be more natural.
> > That would be easy enough. Why don't we simply allow user space to set
> > link mode directly too (in addition to being able to constrain lanes
> > for autoneg or forced speeds)?
>
> It is already possible to do using 'advertise' parameter.
That's not the same thing. If it were, you wouldn't need the lanes
parameter in the first place.
Regards,
Edwin Peer
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4160 bytes --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH net-next v3 1/7] ethtool: Extend link modes settings uAPI with lanes
2021-01-25 15:53 ` Danielle Ratson
@ 2021-01-25 19:02 ` Jakub Kicinski
0 siblings, 0 replies; 31+ messages in thread
From: Jakub Kicinski @ 2021-01-25 19:02 UTC (permalink / raw)
To: Danielle Ratson
Cc: netdev, davem, Jiri Pirko, andrew, f.fainelli, mkubecek, mlxsw,
Ido Schimmel
On Mon, 25 Jan 2021 15:53:24 +0000 Danielle Ratson wrote:
> > > @@ -353,10 +358,39 @@ 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);
> > > +
> > > + if (req_lanes) {
> > > + u32 lanes_cfg = nla_get_u32(tb[ETHTOOL_A_LINKMODES_LANES]);
> >
> > req_lanes == tb[ETHTOOL_A_LINKMODES_LANES], right?
>
> Yes, but req_lanes is a bool and doesn't fit to nla_get_u32. Do you want me to change the req_lanes type and name?
Ah, yes please.
> > Please use req_lanes variable where possible.
> >
> > > +
> > > + if (!is_power_of_2(lanes_cfg)) {
> > > + NL_SET_ERR_MSG_ATTR(info->extack,
> > > + tb[ETHTOOL_A_LINKMODES_LANES],
> > > + "lanes value is invalid");
> > > + return -EINVAL;
> > > + }
> > > +
> > > + /* 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,
> > > + tb[ETHTOOL_A_LINKMODES_LANES],
> > > + "lanes configuration not supported by device");
> > > + return -EOPNOTSUPP;
> > > + }
> >
> > This validation does not depend on the current settings at all,
> > it's just input validation, it can be done before rtnl_lock is
> > taken (in a new function).
> >
> > You can move ethnl_validate_master_slave_cfg() to that function as
> > well (as a cleanup before this patch).
>
> Do you mean to move the ethnl_validate_master_slave_cfg() if from
> that function?
Yes, to a separate helper.
> Doesn't it depend on the current settings, as opposed
> to the supported lanes param that you wanted me to move as well? Not
> sure I understand the second part of the request...
Sorry maybe I quoted a little too much context form the patch.
A helper like this:
static int ethnl_check_linkmodes(...)
{
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;
}
lanes_cfg = ...
if (!is_power_of_2(...lanes_cfg)) {
...
return -EINVAL;
}
return 0;
}
Which you can call before the device reference is taken:
int ethnl_set_linkmodes(struct sk_buff *skb, struct genl_info *info)
{
struct ethtool_link_ksettings ksettings = {};
struct ethnl_req_info req_info = {};
struct nlattr **tb = info->attrs;
struct net_device *dev;
bool mod = false;
int ret;
+ ret = ethnl_check_linkmodes(tb);
+ if (ret)
+ return ret;
ret = ethnl_parse_header_dev_get(&req_info,
tb[ETHTOOL_A_LINKMODES_HEADER],
genl_info_net(info), info->extack,
true);
if (ret < 0)
return ret;
But please make sure that you move the master_slave_cfg check in a
separate patch.
^ permalink raw reply [flat|nested] 31+ messages in thread
* RE: [PATCH net-next v3 2/7] ethtool: Get link mode in use instead of speed and duplex parameters
2021-01-25 18:03 ` Edwin Peer
@ 2021-01-26 17:06 ` Danielle Ratson
2021-01-26 17:14 ` Edwin Peer
0 siblings, 1 reply; 31+ messages in thread
From: Danielle Ratson @ 2021-01-26 17:06 UTC (permalink / raw)
To: Edwin Peer
Cc: netdev, David S . Miller, Jakub Kicinski, Jiri Pirko,
Andrew Lunn, f.fainelli, Michal Kubecek, mlxsw, Ido Schimmel
> -----Original Message-----
> From: Edwin Peer <edwin.peer@broadcom.com>
> Sent: Monday, January 25, 2021 8:04 PM
> To: Danielle Ratson <danieller@nvidia.com>
> Cc: netdev <netdev@vger.kernel.org>; David S . Miller <davem@davemloft.net>; Jakub Kicinski <kuba@kernel.org>; Jiri Pirko
> <jiri@nvidia.com>; Andrew Lunn <andrew@lunn.ch>; f.fainelli@gmail.com; Michal Kubecek <mkubecek@suse.cz>; mlxsw
> <mlxsw@nvidia.com>; Ido Schimmel <idosch@nvidia.com>
> Subject: Re: [PATCH net-next v3 2/7] ethtool: Get link mode in use instead of speed and duplex parameters
>
> On Sun, Jan 24, 2021 at 12:36 AM Danielle Ratson <danieller@nvidia.com> wrote:
>
> > > Why isn't this also handled using a capability bit as is done for
> > > lanes? Is link_mode read-only? Should it / will it always be? If not,
> > > can drivers also derive the other fields if asked to set link_mode?
> >
> > The link_mode param is only for deriving all the speed, lanes and duplex params in ethtool instead of deriving in driver and then >
> passing each individual, as Michal asked.
>
> I understand the benefit of deriving the dependent fields in core code
> rather than in each driver, I just don't think this is necessarily
> mutually exclusive with being able to force a particular link mode at
> the driver API, making link_mode R/W (and even extend this interface
> to user space). For a driver that works internally in terms of the
> link_mode it's returning, this would be more natural.
I am not sure I fully understood you, but it seems like some expansion that can be done in the future if needed, and doesn't need to hold that patchset back.
Thanks,
Danielle
>
> > > That would be easy enough. Why don't we simply allow user space to set
> > > link mode directly too (in addition to being able to constrain lanes
> > > for autoneg or forced speeds)?
> >
> > It is already possible to do using 'advertise' parameter.
>
> That's not the same thing. If it were, you wouldn't need the lanes
> parameter in the first place.
>
> Regards,
> Edwin Peer
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH net-next v3 2/7] ethtool: Get link mode in use instead of speed and duplex parameters
2021-01-26 17:06 ` Danielle Ratson
@ 2021-01-26 17:14 ` Edwin Peer
2021-01-27 13:22 ` Danielle Ratson
0 siblings, 1 reply; 31+ messages in thread
From: Edwin Peer @ 2021-01-26 17:14 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: 1145 bytes --]
On Tue, Jan 26, 2021 at 9:09 AM Danielle Ratson <danieller@nvidia.com> wrote:
> > I understand the benefit of deriving the dependent fields in core code
> > rather than in each driver, I just don't think this is necessarily
> > mutually exclusive with being able to force a particular link mode at
> > the driver API, making link_mode R/W (and even extend this interface
> > to user space). For a driver that works internally in terms of the
> > link_mode it's returning, this would be more natural.
>
> I am not sure I fully understood you, but it seems like some expansion that can be
> done in the future if needed, and doesn't need to hold that patchset back.
For one thing, it's cleaner if the driver API is symmetric. The
proposed solution sets attributes in terms of speeds and lanes, etc.,
but it gets them in terms of a compound link_info. But, this asymmetry
aside, if link_mode may eventually become R/W at the driver API, as
you suggest, then it is more appropriate to guard it with a capability
bit, as has been done for lanes, rather than use the -1 special value
to indicate that the driver did not set it.
Regards,
Edwin Peer
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4160 bytes --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* RE: [PATCH net-next v3 2/7] ethtool: Get link mode in use instead of speed and duplex parameters
2021-01-26 17:14 ` Edwin Peer
@ 2021-01-27 13:22 ` Danielle Ratson
2021-01-28 20:26 ` Michal Kubecek
0 siblings, 1 reply; 31+ messages in thread
From: Danielle Ratson @ 2021-01-27 13:22 UTC (permalink / raw)
To: Edwin Peer, Michal Kubecek
Cc: netdev, David S . Miller, Jakub Kicinski, Jiri Pirko,
Andrew Lunn, f.fainelli, mlxsw, Ido Schimmel
> -----Original Message-----
> From: Edwin Peer <edwin.peer@broadcom.com>
> Sent: Tuesday, January 26, 2021 7:14 PM
> To: Danielle Ratson <danieller@nvidia.com>
> Cc: netdev <netdev@vger.kernel.org>; David S . Miller <davem@davemloft.net>; Jakub Kicinski <kuba@kernel.org>; Jiri Pirko
> <jiri@nvidia.com>; Andrew Lunn <andrew@lunn.ch>; f.fainelli@gmail.com; Michal Kubecek <mkubecek@suse.cz>; mlxsw
> <mlxsw@nvidia.com>; Ido Schimmel <idosch@nvidia.com>
> Subject: Re: [PATCH net-next v3 2/7] ethtool: Get link mode in use instead of speed and duplex parameters
>
> On Tue, Jan 26, 2021 at 9:09 AM Danielle Ratson <danieller@nvidia.com> wrote:
>
> > > I understand the benefit of deriving the dependent fields in core code
> > > rather than in each driver, I just don't think this is necessarily
> > > mutually exclusive with being able to force a particular link mode at
> > > the driver API, making link_mode R/W (and even extend this interface
> > > to user space). For a driver that works internally in terms of the
> > > link_mode it's returning, this would be more natural.
> >
> > I am not sure I fully understood you, but it seems like some expansion that can be
> > done in the future if needed, and doesn't need to hold that patchset back.
>
> For one thing, it's cleaner if the driver API is symmetric. The
> proposed solution sets attributes in terms of speeds and lanes, etc.,
> but it gets them in terms of a compound link_info. But, this asymmetry
> aside, if link_mode may eventually become R/W at the driver API, as
> you suggest, then it is more appropriate to guard it with a capability
> bit, as has been done for lanes, rather than use the -1 special value
> to indicate that the driver did not set it.
>
> Regards,
> Edwin Peer
This patchset adds lanes parameter, not link_mode. The link_mode addition was added as a read-only parameter for the reasons we mentioned, and I am not sure that implementing the symmetric side is relevant for this patchset.
Michal, do you think we will use the Write side of the link_mode parameter? And if so, do you think it is relevant for this specific patchset?
Thanks,
Danielle
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH net-next v3 2/7] ethtool: Get link mode in use instead of speed and duplex parameters
2021-01-27 13:22 ` Danielle Ratson
@ 2021-01-28 20:26 ` Michal Kubecek
2021-01-31 15:33 ` Danielle Ratson
0 siblings, 1 reply; 31+ messages in thread
From: Michal Kubecek @ 2021-01-28 20:26 UTC (permalink / raw)
To: Danielle Ratson
Cc: Edwin Peer, netdev, David S . Miller, Jakub Kicinski, Jiri Pirko,
Andrew Lunn, f.fainelli, mlxsw, Ido Schimmel
On Wed, Jan 27, 2021 at 01:22:02PM +0000, Danielle Ratson wrote:
> > -----Original Message-----
> > From: Edwin Peer <edwin.peer@broadcom.com>
> > Sent: Tuesday, January 26, 2021 7:14 PM
> > To: Danielle Ratson <danieller@nvidia.com>
> > Cc: netdev <netdev@vger.kernel.org>; David S . Miller <davem@davemloft.net>; Jakub Kicinski <kuba@kernel.org>; Jiri Pirko
> > <jiri@nvidia.com>; Andrew Lunn <andrew@lunn.ch>; f.fainelli@gmail.com; Michal Kubecek <mkubecek@suse.cz>; mlxsw
> > <mlxsw@nvidia.com>; Ido Schimmel <idosch@nvidia.com>
> > Subject: Re: [PATCH net-next v3 2/7] ethtool: Get link mode in use instead of speed and duplex parameters
> >
> > For one thing, it's cleaner if the driver API is symmetric. The
> > proposed solution sets attributes in terms of speeds and lanes,
> > etc., but it gets them in terms of a compound link_info. But, this
> > asymmetry aside, if link_mode may eventually become R/W at the
> > driver API, as you suggest, then it is more appropriate to guard it
> > with a capability bit, as has been done for lanes, rather than use
> > the -1 special value to indicate that the driver did not set it.
> >
> > Regards,
> > Edwin Peer
>
> This patchset adds lanes parameter, not link_mode. The link_mode
> addition was added as a read-only parameter for the reasons we
> mentioned, and I am not sure that implementing the symmetric side is
> relevant for this patchset.
>
> Michal, do you think we will use the Write side of the link_mode
> parameter?
It makes sense, IMHO. Unless we also add "media" (or whatever name would
be most appropriate) parameter, we cannot in general fully determine the
link mode by speed, duplex and lanes. And using "advertise" to select
a specific mode with disabled autonegotiation would be rather confusing,
I believe. (At the moment, ethtool does not even support syntax like
"advertise <mode>" but it will be easy to support
"advertise <mode>... [--]" and I think we should extend the syntax to
support it, regardless of what we choose.) So if we want to allow user
to pick a specific link node by name or bit mask (or rather index?),
I would prefer using a separate parameter.
> And if so, do you think it is relevant for this specific
> patchset?
I don't see an obvious problem with leaving that part for later so
I would say it's up to you.
Michal
^ permalink raw reply [flat|nested] 31+ messages in thread
* RE: [PATCH net-next v3 2/7] ethtool: Get link mode in use instead of speed and duplex parameters
2021-01-28 20:26 ` Michal Kubecek
@ 2021-01-31 15:33 ` Danielle Ratson
2021-01-31 17:39 ` Edwin Peer
0 siblings, 1 reply; 31+ messages in thread
From: Danielle Ratson @ 2021-01-31 15:33 UTC (permalink / raw)
To: Michal Kubecek, Edwin Peer
Cc: netdev, David S . Miller, Jakub Kicinski, Jiri Pirko,
Andrew Lunn, f.fainelli, mlxsw, Ido Schimmel
> -----Original Message-----
> From: Michal Kubecek <mkubecek@suse.cz>
> Sent: Thursday, January 28, 2021 10:27 PM
> To: Danielle Ratson <danieller@nvidia.com>
> Cc: Edwin Peer <edwin.peer@broadcom.com>; netdev <netdev@vger.kernel.org>; David S . Miller <davem@davemloft.net>; Jakub
> Kicinski <kuba@kernel.org>; Jiri Pirko <jiri@nvidia.com>; Andrew Lunn <andrew@lunn.ch>; f.fainelli@gmail.com; mlxsw
> <mlxsw@nvidia.com>; Ido Schimmel <idosch@nvidia.com>
> Subject: Re: [PATCH net-next v3 2/7] ethtool: Get link mode in use instead of speed and duplex parameters
>
> On Wed, Jan 27, 2021 at 01:22:02PM +0000, Danielle Ratson wrote:
> > > -----Original Message-----
> > > From: Edwin Peer <edwin.peer@broadcom.com>
> > > Sent: Tuesday, January 26, 2021 7:14 PM
> > > To: Danielle Ratson <danieller@nvidia.com>
> > > Cc: netdev <netdev@vger.kernel.org>; David S . Miller
> > > <davem@davemloft.net>; Jakub Kicinski <kuba@kernel.org>; Jiri Pirko
> > > <jiri@nvidia.com>; Andrew Lunn <andrew@lunn.ch>;
> > > f.fainelli@gmail.com; Michal Kubecek <mkubecek@suse.cz>; mlxsw
> > > <mlxsw@nvidia.com>; Ido Schimmel <idosch@nvidia.com>
> > > Subject: Re: [PATCH net-next v3 2/7] ethtool: Get link mode in use
> > > instead of speed and duplex parameters
> > >
> > > For one thing, it's cleaner if the driver API is symmetric. The
> > > proposed solution sets attributes in terms of speeds and lanes,
> > > etc., but it gets them in terms of a compound link_info. But, this
> > > asymmetry aside, if link_mode may eventually become R/W at the
> > > driver API, as you suggest, then it is more appropriate to guard it
> > > with a capability bit, as has been done for lanes, rather than use
> > > the -1 special value to indicate that the driver did not set it.
> > >
> > > Regards,
> > > Edwin Peer
> >
> > This patchset adds lanes parameter, not link_mode. The link_mode
> > addition was added as a read-only parameter for the reasons we
> > mentioned, and I am not sure that implementing the symmetric side is
> > relevant for this patchset.
> >
> > Michal, do you think we will use the Write side of the link_mode
> > parameter?
>
> It makes sense, IMHO. Unless we also add "media" (or whatever name would be most appropriate) parameter, we cannot in general
> fully determine the link mode by speed, duplex and lanes. And using "advertise" to select a specific mode with disabled
> autonegotiation would be rather confusing, I believe. (At the moment, ethtool does not even support syntax like "advertise <mode>"
> but it will be easy to support "advertise <mode>... [--]" and I think we should extend the syntax to support it, regardless of what we
> choose.) So if we want to allow user to pick a specific link node by name or bit mask (or rather index?), I would prefer using a separate
> parameter.
>
> > And if so, do you think it is relevant for this specific
> > patchset?
>
> I don't see an obvious problem with leaving that part for later so I would say it's up to you.
>
> Michal
Thanks Michal.
Edwin, adding the a new parameter requires a new patchset in my opinion. Implementing the symmetrical side of the link_mode get,
however can be a part of this set. But, the problem with that would be that, as Michal said, speed lanes and duplex can't provide us a single
link_mode because of the media. And since link_mode is one bit parameter, passing it to the driver while randomly choosing the media, may
cause either information loss, or even fail to sync on a link mode if the chosen media is specifically not supported in the lower levels.
So, in my opinion it is related to adding the new parameter we discussed, and should be done in a separate set.
Thanks,
Danielle
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH net-next v3 2/7] ethtool: Get link mode in use instead of speed and duplex parameters
2021-01-31 15:33 ` Danielle Ratson
@ 2021-01-31 17:39 ` Edwin Peer
2021-02-01 13:49 ` Danielle Ratson
0 siblings, 1 reply; 31+ messages in thread
From: Edwin Peer @ 2021-01-31 17:39 UTC (permalink / raw)
To: Danielle Ratson
Cc: Michal Kubecek, netdev, David S . Miller, Jakub Kicinski,
Jiri Pirko, Andrew Lunn, f.fainelli, mlxsw, Ido Schimmel
[-- Attachment #1: Type: text/plain, Size: 1453 bytes --]
On Sun, Jan 31, 2021 at 7:33 AM Danielle Ratson <danieller@nvidia.com> wrote:
> Edwin, adding the a new parameter requires a new patchset in my opinion.
> Implementing the symmetrical side of the link_mode get, however can be a
> part of this set. But, the problem with that would be that, as Michal said,
> speed lanes and duplex can't provide us a single link_mode because of the
> media. And since link_mode is one bit parameter, passing it to the driver
> while randomly choosing the media, may cause either information loss, or
> even fail to sync on a link mode if the chosen media is specifically not
> supported in the lower levels. So, in my opinion it is related to adding the
> new parameter we discussed, and should be done in a separate set.
Media is a little special in the sense that it physically depends on
what's plugged in. In that sense, media is truly read only. Setting it
won't change what's plugged in, so not having a separate knob for it
is probably okay, as it can be inferred from the active link mode on
the query side.
I don't see why the driver can't error if asked to set a link mode
having an incompatible media? The link modes corresponding to media
that's not plugged in wouldn't be listed in the supported set, so
there's no reason the user should expect to be able to set those.
There's no ambiguity or information loss if you refuse to set modes
that don't have the matching media attached.
Regards,
Edwin Peer
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4160 bytes --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* RE: [PATCH net-next v3 2/7] ethtool: Get link mode in use instead of speed and duplex parameters
2021-01-31 17:39 ` Edwin Peer
@ 2021-02-01 13:49 ` Danielle Ratson
2021-02-01 18:14 ` Edwin Peer
0 siblings, 1 reply; 31+ messages in thread
From: Danielle Ratson @ 2021-02-01 13:49 UTC (permalink / raw)
To: Edwin Peer
Cc: Michal Kubecek, netdev, David S . Miller, Jakub Kicinski,
Jiri Pirko, Andrew Lunn, f.fainelli, mlxsw, Ido Schimmel
> -----Original Message-----
> From: Edwin Peer <edwin.peer@broadcom.com>
> Sent: Sunday, January 31, 2021 7:39 PM
> To: Danielle Ratson <danieller@nvidia.com>
> Cc: Michal Kubecek <mkubecek@suse.cz>; netdev <netdev@vger.kernel.org>; David S . Miller <davem@davemloft.net>; Jakub
> Kicinski <kuba@kernel.org>; Jiri Pirko <jiri@nvidia.com>; Andrew Lunn <andrew@lunn.ch>; f.fainelli@gmail.com; mlxsw
> <mlxsw@nvidia.com>; Ido Schimmel <idosch@nvidia.com>
> Subject: Re: [PATCH net-next v3 2/7] ethtool: Get link mode in use instead of speed and duplex parameters
>
> On Sun, Jan 31, 2021 at 7:33 AM Danielle Ratson <danieller@nvidia.com> wrote:
>
> > Edwin, adding the a new parameter requires a new patchset in my opinion.
> > Implementing the symmetrical side of the link_mode get, however can be a
> > part of this set. But, the problem with that would be that, as Michal said,
> > speed lanes and duplex can't provide us a single link_mode because of the
> > media. And since link_mode is one bit parameter, passing it to the driver
> > while randomly choosing the media, may cause either information loss, or
> > even fail to sync on a link mode if the chosen media is specifically not
> > supported in the lower levels. So, in my opinion it is related to adding the
> > new parameter we discussed, and should be done in a separate set.
>
> Media is a little special in the sense that it physically depends on
> what's plugged in. In that sense, media is truly read only. Setting it
> won't change what's plugged in, so not having a separate knob for it
> is probably okay, as it can be inferred from the active link mode on
> the query side.
>
> I don't see why the driver can't error if asked to set a link mode
> having an incompatible media? The link modes corresponding to media
> that's not plugged in wouldn't be listed in the supported set, so
> there's no reason the user should expect to be able to set those.
> There's no ambiguity or information loss if you refuse to set modes
> that don't have the matching media attached.
>
> Regards,
> Edwin Peer
Ok, ill send another version with the symmetrical side. Ethtool will try to compose
a supported link_mode from the parameters from user space and will choose
randomly between the supported ones. Sounds ok?
Thanks,
Danielle
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH net-next v3 2/7] ethtool: Get link mode in use instead of speed and duplex parameters
2021-02-01 13:49 ` Danielle Ratson
@ 2021-02-01 18:14 ` Edwin Peer
2021-02-01 20:29 ` Jakub Kicinski
0 siblings, 1 reply; 31+ messages in thread
From: Edwin Peer @ 2021-02-01 18:14 UTC (permalink / raw)
To: Danielle Ratson
Cc: Michal Kubecek, netdev, David S . Miller, Jakub Kicinski,
Jiri Pirko, Andrew Lunn, f.fainelli, mlxsw, Ido Schimmel
[-- Attachment #1: Type: text/plain, Size: 980 bytes --]
On Mon, Feb 1, 2021 at 5:49 AM Danielle Ratson <danieller@nvidia.com> wrote:
> Ok, ill send another version with the symmetrical side. Ethtool will try to compose
> a supported link_mode from the parameters from user space and will choose
> randomly between the supported ones. Sounds ok?
I think it should be deterministic. It should be possible to select
the appropriate mode either based on the current media type or the
current link mode (which implies a media type). Alternatively, if the
user space request only specifies a subset, such as speed, fall back
to the existing behaviour and don't supply the request to the driver
in the form of a compound link mode in those cases (perhaps indicating
this by not setting the capability bit). The former approach has the
potential to tidy up drivers if we decide that drivers providing the
capability can ignore the other fields and rely solely on link mode,
the latter is no worse than what we have today.
Regards,
Edwin Peer
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4160 bytes --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH net-next v3 2/7] ethtool: Get link mode in use instead of speed and duplex parameters
2021-02-01 18:14 ` Edwin Peer
@ 2021-02-01 20:29 ` Jakub Kicinski
2021-02-01 21:05 ` Edwin Peer
0 siblings, 1 reply; 31+ messages in thread
From: Jakub Kicinski @ 2021-02-01 20:29 UTC (permalink / raw)
To: Edwin Peer
Cc: Danielle Ratson, Michal Kubecek, netdev, David S . Miller,
Jiri Pirko, Andrew Lunn, f.fainelli, mlxsw, Ido Schimmel
On Mon, 1 Feb 2021 10:14:35 -0800 Edwin Peer wrote:
> On Mon, Feb 1, 2021 at 5:49 AM Danielle Ratson <danieller@nvidia.com> wrote:
> > Ok, ill send another version with the symmetrical side. Ethtool
> > will try to compose a supported link_mode from the parameters from
> > user space and will choose randomly between the supported ones.
> > Sounds ok?
>
> I think it should be deterministic. It should be possible to select
> the appropriate mode either based on the current media type or the
> current link mode (which implies a media type). Alternatively, if the
> user space request only specifies a subset, such as speed, fall back
> to the existing behaviour and don't supply the request to the driver
> in the form of a compound link mode in those cases (perhaps indicating
> this by not setting the capability bit). The former approach has the
> potential to tidy up drivers if we decide that drivers providing the
> capability can ignore the other fields and rely solely on link mode,
> the latter is no worse than what we have today.
The media part is beginning to sound concerning. Every time we
under-specify an interface we end up with #vendors different
interpretations. And since HW is programmed by FW in most high
speed devices we can't even review the right thing is done.
At least it's clear what setting a number of lanes means.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH net-next v3 2/7] ethtool: Get link mode in use instead of speed and duplex parameters
2021-02-01 20:29 ` Jakub Kicinski
@ 2021-02-01 21:05 ` Edwin Peer
2021-02-01 21:41 ` Jakub Kicinski
0 siblings, 1 reply; 31+ messages in thread
From: Edwin Peer @ 2021-02-01 21:05 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Danielle Ratson, Michal Kubecek, netdev, David S . Miller,
Jiri Pirko, Andrew Lunn, f.fainelli, mlxsw, Ido Schimmel
[-- Attachment #1: Type: text/plain, Size: 1231 bytes --]
On Mon, Feb 1, 2021 at 12:29 PM Jakub Kicinski <kuba@kernel.org> wrote:
> > I think it should be deterministic. It should be possible to select
> > the appropriate mode either based on the current media type or the
> > current link mode (which implies a media type). Alternatively, if the
> > user space request only specifies a subset, such as speed, fall back
> > to the existing behaviour and don't supply the request to the driver
> > in the form of a compound link mode in those cases (perhaps indicating
> > this by not setting the capability bit). The former approach has the
> > potential to tidy up drivers if we decide that drivers providing the
> > capability can ignore the other fields and rely solely on link mode,
> > the latter is no worse than what we have today.
>
> The media part is beginning to sound concerning. Every time we
> under-specify an interface we end up with #vendors different
> interpretations. And since HW is programmed by FW in most high
> speed devices we can't even review the right thing is done.
Each link mode implies a very specific media type, the kernel can
reject illegal combinations based on the supported bitmask before
calling upon the driver to select it.
Regards,
Edwin Peer
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4160 bytes --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH net-next v3 2/7] ethtool: Get link mode in use instead of speed and duplex parameters
2021-02-01 21:05 ` Edwin Peer
@ 2021-02-01 21:41 ` Jakub Kicinski
2021-02-01 21:59 ` Edwin Peer
0 siblings, 1 reply; 31+ messages in thread
From: Jakub Kicinski @ 2021-02-01 21:41 UTC (permalink / raw)
To: Edwin Peer
Cc: Danielle Ratson, Michal Kubecek, netdev, David S . Miller,
Jiri Pirko, Andrew Lunn, f.fainelli, mlxsw, Ido Schimmel
On Mon, 1 Feb 2021 13:05:10 -0800 Edwin Peer wrote:
> On Mon, Feb 1, 2021 at 12:29 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> > > I think it should be deterministic. It should be possible to select
> > > the appropriate mode either based on the current media type or the
> > > current link mode (which implies a media type). Alternatively, if the
> > > user space request only specifies a subset, such as speed, fall back
> > > to the existing behaviour and don't supply the request to the driver
> > > in the form of a compound link mode in those cases (perhaps indicating
> > > this by not setting the capability bit). The former approach has the
> > > potential to tidy up drivers if we decide that drivers providing the
> > > capability can ignore the other fields and rely solely on link mode,
> > > the latter is no worse than what we have today.
> >
> > The media part is beginning to sound concerning. Every time we
> > under-specify an interface we end up with #vendors different
> > interpretations. And since HW is programmed by FW in most high
> > speed devices we can't even review the right thing is done.
>
> Each link mode implies a very specific media type, the kernel can
> reject illegal combinations based on the supported bitmask before
> calling upon the driver to select it.
Are you talking about validation against a driver-supplied list of
HW-supported modes, or SFP-supported modes for a currently plugged
in module?
If I'm reading prior responses right it is the former.
The concern is around "what happens if user selected nnG-SR4 but user
plugged in nnG-CR4". The MAC/PHY/serdes settings will be identical.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH net-next v3 2/7] ethtool: Get link mode in use instead of speed and duplex parameters
2021-02-01 21:41 ` Jakub Kicinski
@ 2021-02-01 21:59 ` Edwin Peer
2021-02-01 22:20 ` Jakub Kicinski
0 siblings, 1 reply; 31+ messages in thread
From: Edwin Peer @ 2021-02-01 21:59 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Danielle Ratson, Michal Kubecek, netdev, David S . Miller,
Jiri Pirko, Andrew Lunn, f.fainelli, mlxsw, Ido Schimmel
[-- Attachment #1: Type: text/plain, Size: 1256 bytes --]
On Mon, Feb 1, 2021 at 1:41 PM Jakub Kicinski <kuba@kernel.org> wrote:
> > > The media part is beginning to sound concerning. Every time we
> > > under-specify an interface we end up with #vendors different
> > > interpretations. And since HW is programmed by FW in most high
> > > speed devices we can't even review the right thing is done.
> >
> > Each link mode implies a very specific media type, the kernel can
> > reject illegal combinations based on the supported bitmask before
> > calling upon the driver to select it.
>
> Are you talking about validation against a driver-supplied list of
> HW-supported modes, or SFP-supported modes for a currently plugged
> in module?
Should they not be the same thing?
> The concern is around "what happens if user selected nnG-SR4 but user
> plugged in nnG-CR4". The MAC/PHY/serdes settings will be identical.
Yes, there would be multiple link modes that map to the same speed and
lane combination, but that doesn't mean you need to accept them if the
media doesn't match what's plugged in also. In the above scenario, the
supported mask should not list SR because CR is physically plugged in.
That way, the user knows what options are legal and the kernel knows
what it can reject.
Regards,
Edwin Peer
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4160 bytes --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH net-next v3 2/7] ethtool: Get link mode in use instead of speed and duplex parameters
2021-02-01 21:59 ` Edwin Peer
@ 2021-02-01 22:20 ` Jakub Kicinski
2021-02-02 0:14 ` Edwin Peer
0 siblings, 1 reply; 31+ messages in thread
From: Jakub Kicinski @ 2021-02-01 22:20 UTC (permalink / raw)
To: Edwin Peer
Cc: Danielle Ratson, Michal Kubecek, netdev, David S . Miller,
Jiri Pirko, Andrew Lunn, f.fainelli, mlxsw, Ido Schimmel
On Mon, 1 Feb 2021 13:59:45 -0800 Edwin Peer wrote:
> On Mon, Feb 1, 2021 at 1:41 PM Jakub Kicinski <kuba@kernel.org> wrote:
> > > Each link mode implies a very specific media type, the kernel can
> > > reject illegal combinations based on the supported bitmask before
> > > calling upon the driver to select it.
> >
> > Are you talking about validation against a driver-supplied list of
> > HW-supported modes, or SFP-supported modes for a currently plugged
> > in module?
>
> Should they not be the same thing?
Okay. Does "supported modes" in ethtool for bnxt get ANDed with the
supported modes of the plugged in module?
What happens when no module is plugged in? List all?
I've surveyed this behavior a few years back and three vendors I tested
all had different interpretation on what to list in supported modes :/
> > The concern is around "what happens if user selected nnG-SR4 but user
> > plugged in nnG-CR4". The MAC/PHY/serdes settings will be identical.
>
> Yes, there would be multiple link modes that map to the same speed and
> lane combination, but that doesn't mean you need to accept them if the
> media doesn't match what's plugged in also. In the above scenario, the
> supported mask should not list SR because CR is physically plugged in.
> That way, the user knows what options are legal and the kernel knows
> what it can reject.
If the modes depend on what's plugged in - what happens if cable gets
removed? We (you, Danielle, I) can agree what we think is best, but
history teaches us that doesn't matter in long run. We had a similar
conversation when it comes to FEC. There simply is no way for upstream
developers to review the behavior is correct.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH net-next v3 2/7] ethtool: Get link mode in use instead of speed and duplex parameters
2021-02-01 22:20 ` Jakub Kicinski
@ 2021-02-02 0:14 ` Edwin Peer
2021-02-02 1:08 ` Jakub Kicinski
0 siblings, 1 reply; 31+ messages in thread
From: Edwin Peer @ 2021-02-02 0:14 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Danielle Ratson, Michal Kubecek, netdev, David S . Miller,
Jiri Pirko, Andrew Lunn, f.fainelli, mlxsw, Ido Schimmel
[-- Attachment #1: Type: text/plain, Size: 1452 bytes --]
On Mon, Feb 1, 2021 at 2:20 PM Jakub Kicinski <kuba@kernel.org> wrote:
> > Should they not be the same thing?
>
> Okay. Does "supported modes" in ethtool for bnxt get ANDed with the
> supported modes of the plugged in module?
>
> What happens when no module is plugged in? List all?
>
> I've surveyed this behavior a few years back and three vendors I tested
> all had different interpretation on what to list in supported modes :/
Fair enough, I can't argue there. ;)
> > Yes, there would be multiple link modes that map to the same speed and
> > lane combination, but that doesn't mean you need to accept them if the
> > media doesn't match what's plugged in also. In the above scenario, the
> > supported mask should not list SR because CR is physically plugged in.
> > That way, the user knows what options are legal and the kernel knows
> > what it can reject.
>
> If the modes depend on what's plugged in - what happens if cable gets
> removed? We (you, Danielle, I) can agree what we think is best, but
> history teaches us that doesn't matter in long run. We had a similar
> conversation when it comes to FEC. There simply is no way for upstream
> developers to review the behavior is correct.
Given that supported is only defined in the context of autoneg today,
once could still specify. But again, you raise a fair concern.
The asymmetry in interface is still ugly though, you get to decide
which ugly is worse. :P
Regards,
Edwin Peer
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4160 bytes --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH net-next v3 2/7] ethtool: Get link mode in use instead of speed and duplex parameters
2021-02-02 0:14 ` Edwin Peer
@ 2021-02-02 1:08 ` Jakub Kicinski
0 siblings, 0 replies; 31+ messages in thread
From: Jakub Kicinski @ 2021-02-02 1:08 UTC (permalink / raw)
To: Edwin Peer
Cc: Danielle Ratson, Michal Kubecek, netdev, David S . Miller,
Jiri Pirko, Andrew Lunn, f.fainelli, mlxsw, Ido Schimmel
On Mon, 1 Feb 2021 16:14:05 -0800 Edwin Peer wrote:
> > > Yes, there would be multiple link modes that map to the same speed and
> > > lane combination, but that doesn't mean you need to accept them if the
> > > media doesn't match what's plugged in also. In the above scenario, the
> > > supported mask should not list SR because CR is physically plugged in.
> > > That way, the user knows what options are legal and the kernel knows
> > > what it can reject.
> >
> > If the modes depend on what's plugged in - what happens if cable gets
> > removed? We (you, Danielle, I) can agree what we think is best, but
> > history teaches us that doesn't matter in long run. We had a similar
> > conversation when it comes to FEC. There simply is no way for upstream
> > developers to review the behavior is correct.
>
> Given that supported is only defined in the context of autoneg today,
> once could still specify. But again, you raise a fair concern.
>
> The asymmetry in interface is still ugly though, you get to decide
> which ugly is worse. :P
Let's move with this series as is. We can see how prevalent the use of
link_mode is on get side first.
^ permalink raw reply [flat|nested] 31+ messages in thread
* RE: [PATCH net-next v3 1/7] ethtool: Extend link modes settings uAPI with lanes
2021-01-20 22:35 ` Edwin Peer
@ 2021-02-02 18:08 ` Danielle Ratson
0 siblings, 0 replies; 31+ messages in thread
From: Danielle Ratson @ 2021-02-02 18:08 UTC (permalink / raw)
To: Edwin Peer
Cc: netdev, David S . Miller, Jakub Kicinski, Jiri Pirko,
Andrew Lunn, f.fainelli, Michal Kubecek, mlxsw, Ido Schimmel
> -----Original Message-----
> From: Edwin Peer <edwin.peer@broadcom.com>
> Sent: Thursday, January 21, 2021 12:36 AM
> To: Danielle Ratson <danieller@nvidia.com>
> Cc: netdev <netdev@vger.kernel.org>; David S . Miller <davem@davemloft.net>; Jakub Kicinski <kuba@kernel.org>; Jiri Pirko
> <jiri@nvidia.com>; Andrew Lunn <andrew@lunn.ch>; f.fainelli@gmail.com; Michal Kubecek <mkubecek@suse.cz>; mlxsw
> <mlxsw@nvidia.com>; Ido Schimmel <idosch@nvidia.com>
> Subject: Re: [PATCH net-next v3 1/7] ethtool: Extend link modes settings uAPI with lanes
>
> On Wed, Jan 20, 2021 at 3:21 AM Danielle Ratson <danieller@nvidia.com> wrote:
>
> > -#define __DEFINE_LINK_MODE_PARAMS(_speed, _type, _duplex) \
> > +#define __DEFINE_LINK_MODE_PARAMS(_speed, _type, _lanes, _duplex) \
> > [ETHTOOL_LINK_MODE(_speed, _type, _duplex)] = { \
> > .speed = SPEED_ ## _speed, \
> > + .lanes = _lanes, \
> > .duplex = __DUPLEX_ ## _duplex \
> > }
>
> What about:
>
> #define __DECLARE_LINK_MODE_LANES(_type, _lanes) \
> static const u32 __LINK_MODE_LANES_ ## _type = _lanes;
>
> #define __DECLARE_LINK_MODE_LANES_ALL(_type) \
> __DECLARE_LINK_MODE_LANES(_type, 1) \
> __DECLARE_LINK_MODE_LANES(_type ## 2, 2) \
> __DECLARE_LINK_MODE_LANES(_type ## 4, 4) \
> __DECLARE_LINK_MODE_LANES(_type ## 8, 8)
>
> __DECLARE_LINK_MODE_LANES_ALL(CR)
> __DECLARE_LINK_MODE_LANES_ALL(DR)
> __DECLARE_LINK_MODE_LANES_ALL(ER)
> __DECLARE_LINK_MODE_LANES_ALL(KR)
> __DECLARE_LINK_MODE_LANES(KX, 1)
> __DECLARE_LINK_MODE_LANES(KX4, 4)
> __DECLARE_LINK_MODE_LANES_ALL(LR)
> __DECLARE_LINK_MODE_LANES(LR_ER_FR, 1)
> __DECLARE_LINK_MODE_LANES(LR2_ER2_FR2, 2)
> __DECLARE_LINK_MODE_LANES(LR4_ER4_FR4, 4)
> __DECLARE_LINK_MODE_LANES(LR8_ER8_FR8, 8)
> __DECLARE_LINK_MODE_LANES(LRM, 1)
> __DECLARE_LINK_MODE_LANES(MLD2, 2);
> __DECLARE_LINK_MODE_LANES_ALL(SR);
> __DECLARE_LINK_MODE_LANES(T, 1)
> __DECLARE_LINK_MODE_LANES(X, 1)
>
> #define __DEFINE_LINK_MODE_PARAMS(_speed, _type, _duplex)
> [ETHTOOL_LINK_MODE(_speed, _type, _duplex)] = { \
> .speed = SPEED_ ## _speed, \
> .lanes = __LINK_MODE_LANES ## _type, \
>
> instead of specifying lanes for each link mode defined below?
>
> Regards,
> Edwin Peer
Thanks for the advice, due to some checkpatch issues, I used your suggestion with a small change.
Danielle
>
> > 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_LINK_MODE_PARAMS(10, T, 1, Half),
> > + __DEFINE_LINK_MODE_PARAMS(10, T, 1, Full),
> > + __DEFINE_LINK_MODE_PARAMS(100, T, 1, Half),
> > + __DEFINE_LINK_MODE_PARAMS(100, T, 1, Full),
> > + __DEFINE_LINK_MODE_PARAMS(1000, T, 1, Half),
> > + __DEFINE_LINK_MODE_PARAMS(1000, T, 1, 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_LINK_MODE_PARAMS(10000, T, 1, Full),
> > __DEFINE_SPECIAL_MODE_PARAMS(Pause),
> > __DEFINE_SPECIAL_MODE_PARAMS(Asym_Pause),
> > - __DEFINE_LINK_MODE_PARAMS(2500, X, Full),
> > + __DEFINE_LINK_MODE_PARAMS(2500, X, 1, 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),
> > + __DEFINE_LINK_MODE_PARAMS(1000, KX, 1, Full),
> > + __DEFINE_LINK_MODE_PARAMS(10000, KX4, 4, Full),
> > + __DEFINE_LINK_MODE_PARAMS(10000, KR, 1, 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_LINK_MODE_PARAMS(20000, MLD2, 2, Full),
> > + __DEFINE_LINK_MODE_PARAMS(20000, KR2, 2, Full),
> > + __DEFINE_LINK_MODE_PARAMS(40000, KR4, 4, Full),
> > + __DEFINE_LINK_MODE_PARAMS(40000, CR4, 4, Full),
> > + __DEFINE_LINK_MODE_PARAMS(40000, SR4, 4, Full),
> > + __DEFINE_LINK_MODE_PARAMS(40000, LR4, 4, Full),
> > + __DEFINE_LINK_MODE_PARAMS(56000, KR4, 4, Full),
> > + __DEFINE_LINK_MODE_PARAMS(56000, CR4, 4, Full),
> > + __DEFINE_LINK_MODE_PARAMS(56000, SR4, 4, Full),
> > + __DEFINE_LINK_MODE_PARAMS(56000, LR4, 4, Full),
> > + __DEFINE_LINK_MODE_PARAMS(25000, CR, 1, Full),
> > + __DEFINE_LINK_MODE_PARAMS(25000, KR, 1, Full),
> > + __DEFINE_LINK_MODE_PARAMS(25000, SR, 1, Full),
> > + __DEFINE_LINK_MODE_PARAMS(50000, CR2, 2, Full),
> > + __DEFINE_LINK_MODE_PARAMS(50000, KR2, 2, Full),
> > + __DEFINE_LINK_MODE_PARAMS(100000, KR4, 4, Full),
> > + __DEFINE_LINK_MODE_PARAMS(100000, SR4, 4, Full),
> > + __DEFINE_LINK_MODE_PARAMS(100000, CR4, 4, Full),
> > + __DEFINE_LINK_MODE_PARAMS(100000, LR4_ER4, 4, Full),
> > + __DEFINE_LINK_MODE_PARAMS(50000, SR2, 2, Full),
> > + __DEFINE_LINK_MODE_PARAMS(1000, X, 1, Full),
> > + __DEFINE_LINK_MODE_PARAMS(10000, CR, 1, Full),
> > + __DEFINE_LINK_MODE_PARAMS(10000, SR, 1, Full),
> > + __DEFINE_LINK_MODE_PARAMS(10000, LR, 1, Full),
> > + __DEFINE_LINK_MODE_PARAMS(10000, LRM, 1, Full),
> > + __DEFINE_LINK_MODE_PARAMS(10000, ER, 1, Full),
> > + __DEFINE_LINK_MODE_PARAMS(2500, T, 1, Full),
> > + __DEFINE_LINK_MODE_PARAMS(5000, T, 1, 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_LINK_MODE_PARAMS(50000, KR, 1, Full),
> > + __DEFINE_LINK_MODE_PARAMS(50000, SR, 1, Full),
> > + __DEFINE_LINK_MODE_PARAMS(50000, CR, 1, Full),
> > + __DEFINE_LINK_MODE_PARAMS(50000, LR_ER_FR, 1, Full),
> > + __DEFINE_LINK_MODE_PARAMS(50000, DR, 1, Full),
> > + __DEFINE_LINK_MODE_PARAMS(100000, KR2, 2, Full),
> > + __DEFINE_LINK_MODE_PARAMS(100000, SR2, 2, Full),
> > + __DEFINE_LINK_MODE_PARAMS(100000, CR2, 2, Full),
> > + __DEFINE_LINK_MODE_PARAMS(100000, LR2_ER2_FR2, 2, Full),
> > + __DEFINE_LINK_MODE_PARAMS(100000, DR2, 2, Full),
> > + __DEFINE_LINK_MODE_PARAMS(200000, KR4, 4, Full),
> > + __DEFINE_LINK_MODE_PARAMS(200000, SR4, 4, Full),
> > + __DEFINE_LINK_MODE_PARAMS(200000, LR4_ER4_FR4, 4, Full),
> > + __DEFINE_LINK_MODE_PARAMS(200000, DR4, 4, Full),
> > + __DEFINE_LINK_MODE_PARAMS(200000, CR4, 4, Full),
> > + __DEFINE_LINK_MODE_PARAMS(100, T1, 1, Full),
> > + __DEFINE_LINK_MODE_PARAMS(1000, T1, 1, Full),
> > + __DEFINE_LINK_MODE_PARAMS(400000, KR8, 8, Full),
> > + __DEFINE_LINK_MODE_PARAMS(400000, SR8, 8, Full),
> > + __DEFINE_LINK_MODE_PARAMS(400000, LR8_ER8_FR8, 8, Full),
> > + __DEFINE_LINK_MODE_PARAMS(400000, DR8, 8, Full),
> > + __DEFINE_LINK_MODE_PARAMS(400000, CR8, 8, 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),
> > + __DEFINE_LINK_MODE_PARAMS(100000, KR, 1, Full),
> > + __DEFINE_LINK_MODE_PARAMS(100000, SR, 1, Full),
> > + __DEFINE_LINK_MODE_PARAMS(100000, LR_ER_FR, 1, Full),
> > + __DEFINE_LINK_MODE_PARAMS(100000, DR, 1, Full),
> > + __DEFINE_LINK_MODE_PARAMS(100000, CR, 1, Full),
> > + __DEFINE_LINK_MODE_PARAMS(200000, KR2, 2, Full),
> > + __DEFINE_LINK_MODE_PARAMS(200000, SR2, 2, Full),
> > + __DEFINE_LINK_MODE_PARAMS(200000, LR2_ER2_FR2, 2, Full),
> > + __DEFINE_LINK_MODE_PARAMS(200000, DR2, 2, Full),
> > + __DEFINE_LINK_MODE_PARAMS(200000, CR2, 2, Full),
> > + __DEFINE_LINK_MODE_PARAMS(400000, KR4, 4, Full),
> > + __DEFINE_LINK_MODE_PARAMS(400000, SR4, 4, Full),
> > + __DEFINE_LINK_MODE_PARAMS(400000, LR4_ER4_FR4, 4, Full),
> > + __DEFINE_LINK_MODE_PARAMS(400000, DR4, 4, Full),
> > + __DEFINE_LINK_MODE_PARAMS(400000, CR4, 4, Full),
> > + __DEFINE_LINK_MODE_PARAMS(100, FX, 1, Half),
> > + __DEFINE_LINK_MODE_PARAMS(100, FX, 1, Full),
> > };
^ permalink raw reply [flat|nested] 31+ messages in thread
end of thread, other threads:[~2021-02-02 18:12 UTC | newest]
Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-20 9:37 [PATCH net-next v3 0/7] Support setting lanes via ethtool Danielle Ratson
2021-01-20 9:37 ` [PATCH net-next v3 1/7] ethtool: Extend link modes settings uAPI with lanes Danielle Ratson
2021-01-20 22:35 ` Edwin Peer
2021-02-02 18:08 ` Danielle Ratson
2021-01-22 3:44 ` Jakub Kicinski
2021-01-25 15:53 ` Danielle Ratson
2021-01-25 19:02 ` Jakub Kicinski
2021-01-20 9:37 ` [PATCH net-next v3 2/7] ethtool: Get link mode in use instead of speed and duplex parameters Danielle Ratson
2021-01-20 23:39 ` Edwin Peer
2021-01-24 8:36 ` Danielle Ratson
2021-01-25 18:03 ` Edwin Peer
2021-01-26 17:06 ` Danielle Ratson
2021-01-26 17:14 ` Edwin Peer
2021-01-27 13:22 ` Danielle Ratson
2021-01-28 20:26 ` Michal Kubecek
2021-01-31 15:33 ` Danielle Ratson
2021-01-31 17:39 ` Edwin Peer
2021-02-01 13:49 ` Danielle Ratson
2021-02-01 18:14 ` Edwin Peer
2021-02-01 20:29 ` Jakub Kicinski
2021-02-01 21:05 ` Edwin Peer
2021-02-01 21:41 ` Jakub Kicinski
2021-02-01 21:59 ` Edwin Peer
2021-02-01 22:20 ` Jakub Kicinski
2021-02-02 0:14 ` Edwin Peer
2021-02-02 1:08 ` Jakub Kicinski
2021-01-20 9:37 ` [PATCH net-next v3 3/7] ethtool: Expose the number of lanes in use Danielle Ratson
2021-01-20 9:37 ` [PATCH net-next v3 4/7] mlxsw: ethtool: Remove max lanes filtering Danielle Ratson
2021-01-20 9:37 ` [PATCH net-next v3 5/7] mlxsw: ethtool: Add support for setting lanes when autoneg is off Danielle Ratson
2021-01-20 9:37 ` [PATCH net-next v3 6/7] mlxsw: ethtool: Pass link mode in use to ethtool Danielle Ratson
2021-01-20 9:37 ` [PATCH net-next v3 7/7] net: selftests: Add lanes setting test Danielle Ratson
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.