[PATCH net-next 0/2] Implement get_link_ksettings for VXLAN and bridge

[PATCH net-next 0/2] Implement get_link_ksettings for VXLAN and bridge

[Matthias Schiffer]

Mesh routing protocol batman-adv (in particular the new BATMAN_V algorithm)
uses the link speed reported by get_link_ksettings to determine a path
metric for wired links. In the mesh framework Gluon [1], we layer VXLAN
and sometimes bridge interfaces on our Ethernet links.

These patches implement get_link_ksettings for these two interface types.
While this is obviously not accurate for bridges with multiple active
ports, it's much better than having no estimate at all (and in the
particular setup of Gluon, bridges with a single port aren't completely
uncommon).


[1] https://github.com/freifunk-gluon/gluon

Matthias Schiffer (2):
  vxlan: implement get_link_ksettings ethtool method
  bridge: implement get_link_ksettings ethtool method

 drivers/net/vxlan.c    | 24 ++++++++++++++++++++++--
 net/bridge/br_device.c | 37 +++++++++++++++++++++++++++++++++++--
 2 files changed, 57 insertions(+), 4 deletions(-)

-- 
2.24.0

[PATCH net-next 1/2] vxlan: implement get_link_ksettings ethtool method

[Matthias Schiffer]

Similar to VLAN and similar drivers, we can forward get_link_ksettings to
the lower dev if we have one to get meaningful speed/duplex data.

Signed-off-by: Matthias Schiffer <mschiffer@universe-factory.net>
---
 drivers/net/vxlan.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 11f5776affb1..bf04bc2e68c2 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -3175,9 +3175,29 @@ static void vxlan_get_drvinfo(struct net_device *netdev,
 	strlcpy(drvinfo->driver, "vxlan", sizeof(drvinfo->driver));
 }
 
+static int vxlan_get_link_ksettings(struct net_device *dev,
+				    struct ethtool_link_ksettings *cmd)
+{
+	struct vxlan_dev *vxlan = netdev_priv(dev);
+	struct vxlan_rdst *dst = &vxlan->default_dst;
+	struct net_device *lowerdev = __dev_get_by_index(vxlan->net,
+							 dst->remote_ifindex);
+
+	if (!lowerdev) {
+		cmd->base.duplex = DUPLEX_UNKNOWN;
+		cmd->base.port = PORT_OTHER;
+		cmd->base.speed = SPEED_UNKNOWN;
+
+		return 0;
+	}
+
+	return __ethtool_get_link_ksettings(lowerdev, cmd);
+}
+
 static const struct ethtool_ops vxlan_ethtool_ops = {
-	.get_drvinfo	= vxlan_get_drvinfo,
-	.get_link	= ethtool_op_get_link,
+	.get_drvinfo		= vxlan_get_drvinfo,
+	.get_link		= ethtool_op_get_link,
+	.get_link_ksettings	= vxlan_get_link_ksettings,
 };
 
 static struct socket *vxlan_create_sock(struct net *net, bool ipv6,
-- 
2.24.0

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

[Matthias Schiffer]

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

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

[Nikolay Aleksandrov]

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>

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

[David Miller]

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.