Skip to content

Commit

Permalink
Merge branch 'ethtool-link_mode'
Browse files Browse the repository at this point in the history
Danielle Ratson says:

====================
Fix link_mode derived params functionality

Currently, link_mode parameter derives 3 other link parameters, speed,
lanes and duplex, and the derived information is sent to user space.

Few bugs were found in that functionality.
First, some drivers clear the 'ethtool_link_ksettings' struct in their
get_link_ksettings() callback and cause receiving wrong link mode
information in user space. And also, some drivers can report random
values in the 'link_mode' field and cause general protection fault.

Second, the link parameters are only derived in netlink path so in ioctl
path, we don't any reasonable values.

Third, setting 'speed 10000 lanes 1' fails since the lanes parameter
wasn't set for ETHTOOL_LINK_MODE_10000baseR_FEC_BIT.

Patch #1 solves the first two problems by removing link_mode parameter
and deriving the link parameters in driver instead of ethtool.
Patch #2 solves the third one, by setting the lanes parameter for the
link_mode.

v3:
	* Remove the link_mode parameter in the first patch to solve
	  both two issues from patch#1 and patch#2.
	* Add the second patch to solve the third issue.

v2:
	* Add patch #2.
	* Introduce 'cap_link_mode_supported' instead of adding a
	  validity field to 'ethtool_link_ksettings' struct in patch #1.
====================

Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
davem330 committed Apr 7, 2021
2 parents bb58023 + fde32db commit 3cf1482
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 23 deletions.
19 changes: 14 additions & 5 deletions drivers/net/ethernet/mellanox/mlxsw/spectrum_ethtool.c
Original file line number Diff line number Diff line change
Expand Up @@ -1230,16 +1230,22 @@ mlxsw_sp1_from_ptys_link_mode(struct mlxsw_sp *mlxsw_sp, bool carrier_ok,
u32 ptys_eth_proto,
struct ethtool_link_ksettings *cmd)
{
struct mlxsw_sp1_port_link_mode link;
int i;

cmd->link_mode = -1;
cmd->base.speed = SPEED_UNKNOWN;
cmd->base.duplex = DUPLEX_UNKNOWN;
cmd->lanes = 0;

if (!carrier_ok)
return;

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;
if (ptys_eth_proto & mlxsw_sp1_port_link_mode[i].mask) {
link = mlxsw_sp1_port_link_mode[i];
ethtool_params_from_link_mode(cmd,
link.mask_ethtool);
}
}
}

Expand Down Expand Up @@ -1672,15 +1678,18 @@ mlxsw_sp2_from_ptys_link_mode(struct mlxsw_sp *mlxsw_sp, bool carrier_ok,
struct mlxsw_sp2_port_link_mode link;
int i;

cmd->link_mode = -1;
cmd->base.speed = SPEED_UNKNOWN;
cmd->base.duplex = DUPLEX_UNKNOWN;
cmd->lanes = 0;

if (!carrier_ok)
return;

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];
ethtool_params_from_link_mode(cmd,
link.mask_ethtool[1]);
}
}
}
Expand Down
9 changes: 8 additions & 1 deletion include/linux/ethtool.h
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,6 @@ struct ethtool_link_ksettings {
__ETHTOOL_DECLARE_LINK_MODE_MASK(lp_advertising);
} link_modes;
u32 lanes;
enum ethtool_link_mode_bit_indices link_mode;
};

/**
Expand Down Expand Up @@ -574,4 +573,12 @@ struct ethtool_phy_ops {
*/
void ethtool_set_ethtool_phy_ops(const struct ethtool_phy_ops *ops);

/*
* ethtool_params_from_link_mode - Derive link parameters from a given link mode
* @link_ksettings: Link parameters to be derived from the link mode
* @link_mode: Link mode
*/
void
ethtool_params_from_link_mode(struct ethtool_link_ksettings *link_ksettings,
enum ethtool_link_mode_bit_indices link_mode);
#endif /* _LINUX_ETHTOOL_H */
17 changes: 17 additions & 0 deletions net/ethtool/common.c
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,7 @@ const struct link_mode_info link_mode_params[] = {
__DEFINE_LINK_MODE_PARAMS(10000, KR, Full),
[ETHTOOL_LINK_MODE_10000baseR_FEC_BIT] = {
.speed = SPEED_10000,
.lanes = 1,
.duplex = DUPLEX_FULL,
},
__DEFINE_LINK_MODE_PARAMS(20000, MLD2, Full),
Expand Down Expand Up @@ -562,3 +563,19 @@ void ethtool_set_ethtool_phy_ops(const struct ethtool_phy_ops *ops)
rtnl_unlock();
}
EXPORT_SYMBOL_GPL(ethtool_set_ethtool_phy_ops);

void
ethtool_params_from_link_mode(struct ethtool_link_ksettings *link_ksettings,
enum ethtool_link_mode_bit_indices link_mode)
{
const struct link_mode_info *link_info;

if (WARN_ON_ONCE(link_mode >= __ETHTOOL_LINK_MODE_MASK_NBITS))
return;

link_info = &link_mode_params[link_mode];
link_ksettings->base.speed = link_info->speed;
link_ksettings->lanes = link_info->lanes;
link_ksettings->base.duplex = link_info->duplex;
}
EXPORT_SYMBOL_GPL(ethtool_params_from_link_mode);
18 changes: 1 addition & 17 deletions net/ethtool/ioctl.c
Original file line number Diff line number Diff line change
Expand Up @@ -426,29 +426,13 @@ 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));

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;
return dev->ethtool_ops->get_link_ksettings(dev, link_ksettings);
}
EXPORT_SYMBOL(__ethtool_get_link_ksettings);

Expand Down

0 comments on commit 3cf1482

Please sign in to comment.