Re: [PATCH net-next 2/2] bridge: implement get_link_ksettings ethtool method
From: Nikolay Aleksandrov <hidden>
Date: 2019-11-11 22:15:33
Also in:
lkml
On 09/11/2019 19:54, Matthias Schiffer wrote:
quoted hunk ↗ jump to hunk
We return the maximum speed of all active ports. This matches how the link speed would give an upper limit for traffic to/from any single peer if the bridge were replaced with a hardware switch. Signed-off-by: Matthias Schiffer <redacted> --- net/bridge/br_device.c | 37 +++++++++++++++++++++++++++++++++++-- 1 file changed, 35 insertions(+), 2 deletions(-)diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c index e804a3016902..e8a2d0c13592 100644 --- a/net/bridge/br_device.c +++ b/net/bridge/br_device.c@@ -263,6 +263,38 @@ static void br_getinfo(struct net_device *dev, struct ethtool_drvinfo *info) strlcpy(info->bus_info, "N/A", sizeof(info->bus_info)); } +static int br_get_link_ksettings(struct net_device *dev, + struct ethtool_link_ksettings *cmd) +{ + struct net_bridge *br = netdev_priv(dev); + struct net_bridge_port *p; + + cmd->base.duplex = DUPLEX_UNKNOWN; + cmd->base.port = PORT_OTHER; + cmd->base.speed = SPEED_UNKNOWN; + + list_for_each_entry(p, &br->port_list, list) { + struct ethtool_link_ksettings ecmd; + struct net_device *pdev = p->dev; + + if (!netif_running(pdev) || !netif_oper_up(pdev)) + continue; + + if (__ethtool_get_link_ksettings(pdev, &ecmd)) + continue; + + if (ecmd.base.speed == (__u32)SPEED_UNKNOWN) + continue; + + if (cmd->base.speed == (__u32)SPEED_UNKNOWN || + cmd->base.speed < ecmd.base.speed) { + cmd->base.speed = ecmd.base.speed; + } + } + + return 0; +} + static netdev_features_t br_fix_features(struct net_device *dev, netdev_features_t features) {@@ -365,8 +397,9 @@ static int br_del_slave(struct net_device *dev, struct net_device *slave_dev) } static const struct ethtool_ops br_ethtool_ops = { - .get_drvinfo = br_getinfo, - .get_link = ethtool_op_get_link, + .get_drvinfo = br_getinfo, + .get_link = ethtool_op_get_link, + .get_link_ksettings = br_get_link_ksettings, }; static const struct net_device_ops br_netdev_ops = {
Code-wise this is ok, but it could cause potential issues because devices will suddenly have speed (without duplex), and macvlan/8021q are examples which use the lowerdev's get_link_ksettings. I searched for potential problems with some user-space software but couldn't find any, so I hope it will be fine. :) Let's see where this goes. Acked-by: Nikolay Aleksandrov <redacted>