* [PATCH, net-next, v4, 1/2] ethtool: Factored out similar ethtool link settings for virtual devices to core
2020-02-13 20:14 [PATCH, net-next, v4, 0/2] net/ethtool: Introduce link_ksettings API for virtual network devices Cris Forno
@ 2020-02-13 20:14 ` Cris Forno
2020-02-14 16:20 ` Jakub Kicinski
2020-02-13 20:14 ` [PATCH, net-next, v4, 2/2] net/ethtool: Introduce link_ksettings API for virtual network devices Cris Forno
1 sibling, 1 reply; 4+ messages in thread
From: Cris Forno @ 2020-02-13 20:14 UTC (permalink / raw)
To: netdev
Cc: mst, jasowang, haiyangz, sthemmin, sashal, tlfalcon, davem,
mkubecek, willemdebruijn.kernel, Cris Forno
Three virtual devices (ibmveth, virtio_net, and netvsc) all have
similar code to get link settings and validate ethtool command. To
eliminate duplication of code, it is factored out into core/ethtool.c.
Signed-off-by: Cris Forno <cforno12@linux.vnet.ibm.com>
---
include/linux/ethtool.h | 7 +++++++
net/ethtool/ioctl.c | 40 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 47 insertions(+)
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 95991e43..149f982 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -394,6 +394,8 @@ struct ethtool_ops {
struct ethtool_coalesce *);
int (*set_per_queue_coalesce)(struct net_device *, u32,
struct ethtool_coalesce *);
+ bool (*virtdev_validate_link_ksettings)(const struct
+ ethtool_link_ksettings *);
int (*get_link_ksettings)(struct net_device *,
struct ethtool_link_ksettings *);
int (*set_link_ksettings)(struct net_device *,
@@ -420,4 +422,9 @@ struct ethtool_rx_flow_rule *
ethtool_rx_flow_rule_create(const struct ethtool_rx_flow_spec_input *input);
void ethtool_rx_flow_rule_destroy(struct ethtool_rx_flow_rule *rule);
+bool ethtool_virtdev_validate_cmd(const struct ethtool_link_ksettings *cmd);
+int ethtool_virtdev_set_link_ksettings(struct net_device *dev,
+ const struct ethtool_link_ksettings *cmd,
+ u32 *dev_speed, u8 *dev_duplex);
+
#endif /* _LINUX_ETHTOOL_H */
diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
index b987052..88b1a42 100644
--- a/net/ethtool/ioctl.c
+++ b/net/ethtool/ioctl.c
@@ -459,6 +459,25 @@ static int load_link_ksettings_from_user(struct ethtool_link_ksettings *to,
return 0;
}
+/* Check if the user is trying to change anything besides speed/duplex */
+bool ethtool_virtdev_validate_cmd(const struct ethtool_link_ksettings *cmd)
+{
+ struct ethtool_link_settings base2 = {};
+
+ base2.speed = cmd->base.speed;
+ base2.port = PORT_OTHER;
+ base2.duplex = cmd->base.duplex;
+ base2.cmd = cmd->base.cmd;
+ base2.link_mode_masks_nwords = cmd->base.link_mode_masks_nwords;
+
+ return !memcmp(&base2, &cmd->base, sizeof(base2)) &&
+ bitmap_empty(cmd->link_modes.supported,
+ __ETHTOOL_LINK_MODE_MASK_NBITS) &&
+ bitmap_empty(cmd->link_modes.lp_advertising,
+ __ETHTOOL_LINK_MODE_MASK_NBITS);
+}
+EXPORT_SYMBOL(ethtool_virtdev_validate_cmd);
+
/* convert a kernel internal ethtool_link_ksettings to
* ethtool_link_usettings in user space. return 0 on success, errno on
* error.
@@ -581,6 +600,27 @@ static int ethtool_set_link_ksettings(struct net_device *dev,
return err;
}
+int ethtool_virtdev_set_link_ksettings(struct net_device *dev,
+ const struct ethtool_link_ksettings *cmd,
+ u32 *dev_speed, u8 *dev_duplex)
+{
+ u32 speed;
+ u8 duplex;
+
+ speed = cmd->base.speed;
+ duplex = cmd->base.duplex;
+ /* don't allow custom speed and duplex */
+ if (!ethtool_validate_speed(speed) ||
+ !ethtool_validate_duplex(duplex) ||
+ !dev->ethtool_ops->virtdev_validate_link_ksettings(cmd))
+ return -EINVAL;
+ *dev_speed = speed;
+ *dev_duplex = duplex;
+
+ return 0;
+}
+EXPORT_SYMBOL(ethtool_virtdev_set_link_ksettings);
+
/* Query device for its ethtool_cmd settings.
*
* Backward compatibility note: for compatibility with legacy ethtool, this is
--
1.8.3.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH, net-next, v4, 2/2] net/ethtool: Introduce link_ksettings API for virtual network devices
2020-02-13 20:14 [PATCH, net-next, v4, 0/2] net/ethtool: Introduce link_ksettings API for virtual network devices Cris Forno
2020-02-13 20:14 ` [PATCH, net-next, v4, 1/2] ethtool: Factored out similar ethtool link settings for virtual devices to core Cris Forno
@ 2020-02-13 20:14 ` Cris Forno
1 sibling, 0 replies; 4+ messages in thread
From: Cris Forno @ 2020-02-13 20:14 UTC (permalink / raw)
To: netdev
Cc: mst, jasowang, haiyangz, sthemmin, sashal, tlfalcon, davem,
mkubecek, willemdebruijn.kernel, Cris Forno
With get/set link settings functions in core/ethtool.c, ibmveth,
netvsc, and virtio now use the core's helper function.
Funtionality changes that pertain to ibmveth driver include:
1. Changed the initial hardcoded link speed to 1GB.
2. Added support for allowing a user to change the reported link
speed via ethtool.
Changes to the netvsc driver include:
1. When netvsc_get_link_ksettings is called, it will defer to the VF
device if it exists to pull accelerated networking values, otherwise
pull default or user-defined values.
2. Similarly, if netvsc_set_link_ksettings called and a VF device
exists, the real values of speed and duplex are changed.
Signed-off-by: Cris Forno <cforno12@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmveth.c | 62 ++++++++++++++++++++++----------------
drivers/net/ethernet/ibm/ibmveth.h | 3 ++
drivers/net/hyperv/netvsc_drv.c | 25 +++++++++------
drivers/net/virtio_net.c | 40 ++----------------------
4 files changed, 58 insertions(+), 72 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmveth.c b/drivers/net/ethernet/ibm/ibmveth.c
index 84121aa..6abac2a 100644
--- a/drivers/net/ethernet/ibm/ibmveth.c
+++ b/drivers/net/ethernet/ibm/ibmveth.c
@@ -712,31 +712,38 @@ static int ibmveth_close(struct net_device *netdev)
return 0;
}
-static int netdev_get_link_ksettings(struct net_device *dev,
- struct ethtool_link_ksettings *cmd)
+static int ibmveth_set_link_ksettings(struct net_device *dev,
+ const struct ethtool_link_ksettings *cmd)
{
- u32 supported, advertising;
-
- supported = (SUPPORTED_1000baseT_Full | SUPPORTED_Autoneg |
- SUPPORTED_FIBRE);
- advertising = (ADVERTISED_1000baseT_Full | ADVERTISED_Autoneg |
- ADVERTISED_FIBRE);
- cmd->base.speed = SPEED_1000;
- cmd->base.duplex = DUPLEX_FULL;
- cmd->base.port = PORT_FIBRE;
- cmd->base.phy_address = 0;
- cmd->base.autoneg = AUTONEG_ENABLE;
-
- ethtool_convert_legacy_u32_to_link_mode(cmd->link_modes.supported,
- supported);
- ethtool_convert_legacy_u32_to_link_mode(cmd->link_modes.advertising,
- advertising);
+ struct ibmveth_adapter *adapter = netdev_priv(dev);
+
+ return ethtool_virtdev_set_link_ksettings(dev, cmd,
+ &adapter->speed,
+ &adapter->duplex);
+}
+
+static int ibmveth_get_link_ksettings(struct net_device *dev,
+ struct ethtool_link_ksettings *cmd)
+{
+ struct ibmveth_adapter *adapter = netdev_priv(dev);
+
+ cmd->base.speed = adapter->speed;
+ cmd->base.duplex = adapter->duplex;
+ cmd->base.port = PORT_OTHER;
return 0;
}
-static void netdev_get_drvinfo(struct net_device *dev,
- struct ethtool_drvinfo *info)
+static void ibmveth_init_link_settings(struct net_device *dev)
+{
+ struct ibmveth_adapter *adapter = netdev_priv(dev);
+
+ adapter->speed = SPEED_1000;
+ adapter->duplex = DUPLEX_FULL;
+}
+
+static void ibmveth_get_drvinfo(struct net_device *dev,
+ struct ethtool_drvinfo *info)
{
strlcpy(info->driver, ibmveth_driver_name, sizeof(info->driver));
strlcpy(info->version, ibmveth_driver_version, sizeof(info->version));
@@ -965,12 +972,14 @@ static void ibmveth_get_ethtool_stats(struct net_device *dev,
}
static const struct ethtool_ops netdev_ethtool_ops = {
- .get_drvinfo = netdev_get_drvinfo,
- .get_link = ethtool_op_get_link,
- .get_strings = ibmveth_get_strings,
- .get_sset_count = ibmveth_get_sset_count,
- .get_ethtool_stats = ibmveth_get_ethtool_stats,
- .get_link_ksettings = netdev_get_link_ksettings,
+ .get_drvinfo = ibmveth_get_drvinfo,
+ .get_link = ethtool_op_get_link,
+ .get_strings = ibmveth_get_strings,
+ .get_sset_count = ibmveth_get_sset_count,
+ .get_ethtool_stats = ibmveth_get_ethtool_stats,
+ .get_link_ksettings = ibmveth_get_link_ksettings,
+ .set_link_ksettings = ibmveth_set_link_ksettings,
+ .virtdev_validate_link_ksettings = ethtool_virtdev_validate_cmd,
};
static int ibmveth_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
@@ -1674,6 +1683,7 @@ static int ibmveth_probe(struct vio_dev *dev, const struct vio_device_id *id)
adapter->netdev = netdev;
adapter->mcastFilterSize = be32_to_cpu(*mcastFilterSize_p);
adapter->pool_config = 0;
+ ibmveth_init_link_settings(netdev);
netif_napi_add(netdev, &adapter->napi, ibmveth_poll, 16);
diff --git a/drivers/net/ethernet/ibm/ibmveth.h b/drivers/net/ethernet/ibm/ibmveth.h
index 4e9bf34..27dfff2 100644
--- a/drivers/net/ethernet/ibm/ibmveth.h
+++ b/drivers/net/ethernet/ibm/ibmveth.h
@@ -162,6 +162,9 @@ struct ibmveth_adapter {
u64 tx_send_failed;
u64 tx_large_packets;
u64 rx_large_packets;
+ /* Ethtool settings */
+ u8 duplex;
+ u32 speed;
};
/*
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 65e12cb..6d8bb85 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -1175,6 +1175,12 @@ static int netvsc_get_link_ksettings(struct net_device *dev,
struct ethtool_link_ksettings *cmd)
{
struct net_device_context *ndc = netdev_priv(dev);
+ struct net_device *vf_netdev;
+
+ vf_netdev = rtnl_dereference(ndc->vf_netdev);
+
+ if (vf_netdev)
+ return __ethtool_get_link_ksettings(vf_netdev, cmd);
cmd->base.speed = ndc->speed;
cmd->base.duplex = ndc->duplex;
@@ -1187,18 +1193,18 @@ static int netvsc_set_link_ksettings(struct net_device *dev,
const struct ethtool_link_ksettings *cmd)
{
struct net_device_context *ndc = netdev_priv(dev);
- u32 speed;
+ struct net_device *vf_netdev = rtnl_dereference(ndc->vf_netdev);
- speed = cmd->base.speed;
- if (!ethtool_validate_speed(speed) ||
- !ethtool_validate_duplex(cmd->base.duplex) ||
- !netvsc_validate_ethtool_ss_cmd(cmd))
- return -EINVAL;
+ if (vf_netdev) {
+ if (!vf_netdev->ethtool_ops->set_link_ksettings)
+ return -EOPNOTSUPP;
- ndc->speed = speed;
- ndc->duplex = cmd->base.duplex;
+ return vf_netdev->ethtool_ops->set_link_ksettings(vf_netdev,
+ cmd);
+ }
- return 0;
+ return ethtool_virtdev_set_link_ksettings(dev, cmd,
+ &ndc->speed, &ndc->duplex);
}
static int netvsc_change_mtu(struct net_device *ndev, int mtu)
@@ -1981,6 +1987,7 @@ static void netvsc_set_msglevel(struct net_device *ndev, u32 val)
.set_link_ksettings = netvsc_set_link_ksettings,
.get_ringparam = netvsc_get_ringparam,
.set_ringparam = netvsc_set_ringparam,
+ .virtdev_validate_link_ksettings = netvsc_validate_ethtool_ss_cmd,
};
static const struct net_device_ops device_ops = {
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 2fe7a31..264ce9f 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -2166,48 +2166,13 @@ static void virtnet_get_channels(struct net_device *dev,
channels->other_count = 0;
}
-/* Check if the user is trying to change anything besides speed/duplex */
-static bool
-virtnet_validate_ethtool_cmd(const struct ethtool_link_ksettings *cmd)
-{
- struct ethtool_link_ksettings diff1 = *cmd;
- struct ethtool_link_ksettings diff2 = {};
-
- /* cmd is always set so we need to clear it, validate the port type
- * and also without autonegotiation we can ignore advertising
- */
- diff1.base.speed = 0;
- diff2.base.port = PORT_OTHER;
- ethtool_link_ksettings_zero_link_mode(&diff1, advertising);
- diff1.base.duplex = 0;
- diff1.base.cmd = 0;
- diff1.base.link_mode_masks_nwords = 0;
-
- return !memcmp(&diff1.base, &diff2.base, sizeof(diff1.base)) &&
- bitmap_empty(diff1.link_modes.supported,
- __ETHTOOL_LINK_MODE_MASK_NBITS) &&
- bitmap_empty(diff1.link_modes.advertising,
- __ETHTOOL_LINK_MODE_MASK_NBITS) &&
- bitmap_empty(diff1.link_modes.lp_advertising,
- __ETHTOOL_LINK_MODE_MASK_NBITS);
-}
-
static int virtnet_set_link_ksettings(struct net_device *dev,
const struct ethtool_link_ksettings *cmd)
{
struct virtnet_info *vi = netdev_priv(dev);
- u32 speed;
-
- speed = cmd->base.speed;
- /* don't allow custom speed and duplex */
- if (!ethtool_validate_speed(speed) ||
- !ethtool_validate_duplex(cmd->base.duplex) ||
- !virtnet_validate_ethtool_cmd(cmd))
- return -EINVAL;
- vi->speed = speed;
- vi->duplex = cmd->base.duplex;
- return 0;
+ return ethtool_virtdev_set_link_ksettings(dev, cmd,
+ &vi->speed, &vi->duplex);
}
static int virtnet_get_link_ksettings(struct net_device *dev,
@@ -2309,6 +2274,7 @@ static void virtnet_update_settings(struct virtnet_info *vi)
.set_link_ksettings = virtnet_set_link_ksettings,
.set_coalesce = virtnet_set_coalesce,
.get_coalesce = virtnet_get_coalesce,
+ .virtdev_validate_link_ksettings = ethtool_virtdev_validate_cmd,
};
static void virtnet_freeze_down(struct virtio_device *vdev)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 4+ messages in thread