diff mbox series

[net-next,2/2] bridge: implement get_link_ksettings ethtool method

Message ID 8e414e98928aba7f11ea997498fb18af05434f5f.1573321597.git.mschiffer@universe-factory.net
State Changes Requested
Delegated to: David Miller
Headers show
Series Implement get_link_ksettings for VXLAN and bridge | expand

Commit Message

Matthias Schiffer Nov. 9, 2019, 5:54 p.m. UTC
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 <mschiffer@universe-factory.net>
---
 net/bridge/br_device.c | 37 +++++++++++++++++++++++++++++++++++--
 1 file changed, 35 insertions(+), 2 deletions(-)

Comments

Nikolay Aleksandrov Nov. 11, 2019, 10:15 p.m. UTC | #1
On 09/11/2019 19:54, Matthias Schiffer wrote:
> 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 <mschiffer@universe-factory.net>
> ---
>  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 <nikolay@cumulusnetworks.com>
David Miller Nov. 11, 2019, 10:41 p.m. UTC | #2
From: Matthias Schiffer <mschiffer@universe-factory.net>
Date: Sat,  9 Nov 2019 18:54:14 +0100

> +		if (cmd->base.speed == (__u32)SPEED_UNKNOWN ||
> +		    cmd->base.speed < ecmd.base.speed) {
> +			cmd->base.speed = ecmd.base.speed;
> +		}

Curly braces are unnecessary for single line statements, so please remove.
diff mbox series

Patch

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 = {