virtio_net: allow hypervisor to indicate linkspeed and duplex setting
Subsystems (4)
v3 cover virtio_net: allow hypervisor to indicate linkspeed and duplex setting
We have found it useful to be able to set the linkspeed and duplex settings from the host-side for virtio_net. This obviates the need for guest changes and settings for these fields, and does not require custom ethtool commands for virtio_net.
The ability to set linkspeed and duplex is useful in various cases as described here:
16032be virtio_net: add ethtool support for set and get of settings Using 'ethtool -s' continues to over-write the linkspeed/duplex settings with this patch.
The 1/3 patch is against net-next, while the 2-3/3 patch are the associated qemu changes that would go in after as update-linux-headers.sh should be run first. So the qemu patches are a demonstration of how I intend this to work.
Thanks,
-Jason
linux changes:
changes from v2:
* move speed/duplex read into virtnet_config_changed_work() so link up changes are detected
Jason Baron (1):
virtio_net: propagate linkspeed/duplex settings from the hypervisor
drivers/net/virtio_net.c | 19 ++++++++++++++++++- include/uapi/linux/virtio_net.h | 13 +++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-)
qemu changes:
changes from v2:
* if link up return configured speed/duplex, else return UNKNOWN speed and duplex
Jason Baron (2):
qemu: virtio-net: use 64-bit values for feature flags
qemu: add linkspeed and duplex settings to virtio-net
hw/net/virtio-net.c | 89 ++++++++++++++++++++--------- include/hw/virtio/virtio-net.h | 5 +- include/standard-headers/linux/virtio_net.h | 13 +++++ 3 files changed, 79 insertions(+), 28 deletions(-)
Signature · 1 line
2.6.1
PATCH v3 1/3 virtio_net: propagate linkspeed/duplex settings from the hypervisor
The ability to set speed and duplex for virtio_net is useful in various scenarios as described here:
16032be virtio_net: add ethtool support for set and get of settings
However, it would be nice to be able to set this from the hypervisor, such that virtio_net doesn't require custom guest ethtool commands.
Introduce a new feature flag, VIRTIO_NET_F_SPEED_DUPLEX, which allows the hypervisor to export a linkspeed and duplex setting. The user can subsequently overwrite it later if desired via: 'ethtool -s'.
Note that VIRTIO_NET_F_SPEED_DUPLEX is defined as bit 63, the intention is that device feature bits are to grow down from bit 63, since the transports are starting from bit 24 and growing up.
--- drivers/net/virtio_net.c | 19 ++++++++++++++++++- include/uapi/linux/virtio_net.h | 13 +++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-)
drivers/net/virtio_net.c +18 -1
@@ -2146,6 +2146,22 @@ static void virtnet_config_changed_work(struct work_struct *work) vi->status = v; +if (virtio_has_feature (vi->vdev,VIRTIO_NET_F_SPEED_DUPLEX )) { +u32 speed; +u8 duplex; + + speed =virtio_cread32 (vi->vdev, + offsetof(struct virtio_net_config , + speed)); +if (ethtool_validate_speed (speed)) + vi->speed = speed; + duplex =virtio_cread8 (vi->vdev, + offsetof(struct virtio_net_config , + duplex)); +if (ethtool_validate_duplex (duplex)) + vi->duplex = duplex; + } +if (vi->status &VIRTIO_NET_S_LINK_UP ) {netif_carrier_on (vi->dev);netif_tx_wake_all_queues (vi->dev);
@@ -2796,7 +2812,8 @@ static struct virtio_device_id id_table[] = { VIRTIO_NET_F_CTRL_RX, VIRTIO_NET_F_CTRL_VLAN, \VIRTIO_NET_F_GUEST_ANNOUNCE ,VIRTIO_NET_F_MQ , \VIRTIO_NET_F_CTRL_MAC_ADDR , \ - VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS +VIRTIO_NET_F_MTU ,VIRTIO_NET_F_CTRL_GUEST_OFFLOADS , \ +VIRTIO_NET_F_SPEED_DUPLEX static unsigned int features[] = {VIRTNET_FEATURES ,
include/uapi/linux/virtio_net.h +13 -0
@@ -57,6 +57,8 @@* Steering */ #define VIRTIO_NET_F_CTRL_MAC_ADDR 23/* Set MAC address */ +#define VIRTIO_NET_F_SPEED_DUPLEX 63/* Device set linkspeed and duplex */ +#ifndef VIRTIO_NET_NO_LEGACY #define VIRTIO_NET_F_GSO 6/* Host handles pkts w/ any GSO type */ #endif /* VIRTIO_NET_NO_LEGACY */
@@ -76,6 +78,17 @@ struct virtio_net_config {__u16 max_virtqueue_pairs;/* Default maximum transmit unit advice */ __u16 mtu; +/* +* speed, in units of 1Mb. All values 0 to INT_MAX are legal. +* Any other value stands for unknown. +*/ +__u32 speed; +/* +* 0x00 - half duplex +* 0x01 - full duplex +* Any other value stands for unknown. +*/ +__u8 duplex; } __attribute__((packed));/*
Signature · 1 line
2.6.1
Michael S. Tsirkin
maintainerRe: [PATCH net-next v3 1/3] virtio_net: propagate linkspeed/duplex settings from the hypervisor
Quoted 53 lines · Jason Baron
The ability to set speed and duplex for
virtio_netis useful in various scenarios as described here:16032be
virtio_net: add ethtool support for set and get of settingsHowever, it would be nice to be able to set this from the hypervisor, such that
virtio_netdoesn't require custom guest ethtool commands.Introduce a new feature flag,
VIRTIO_NET_F_SPEED_DUPLEX, which allows the hypervisor to export a linkspeed and duplex setting. The user can subsequently overwrite it later if desired via: 'ethtool -s'.Note that
VIRTIO_NET_F_SPEED_DUPLEXis defined as bit 63, the intention is that device feature bits are to grow down from bit 63, since the transports are starting from bit 24 and growing up.Signed-off-by Jason Baron <[email protected]>Cc "Michael S. Tsirkin" <[email protected]>Cc Jason Wang <[email protected]>--- drivers/net/virtio_net.c | 19 ++++++++++++++++++- include/uapi/linux/virtio_net.h | 13 +++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-)drivers/net/virtio_net.c +16
-0@@ -2146,6 +2146,22 @@ static void virtnet_config_changed_work(struct work_struct *work) vi->status = v; +if (virtio_has_feature (vi->vdev,VIRTIO_NET_F_SPEED_DUPLEX )) { +u32 speed; +u8 duplex; + + speed =virtio_cread32 (vi->vdev, + offsetof(struct virtio_net_config , + speed)); +if (ethtool_validate_speed (speed)) + vi->speed = speed; + duplex =virtio_cread8 (vi->vdev, + offsetof(struct virtio_net_config , + duplex)); +if (ethtool_validate_duplex (duplex)) + vi->duplex = duplex; + } +if (vi->status &VIRTIO_NET_S_LINK_UP ) {netif_carrier_on (vi->dev);netif_tx_wake_all_queues (vi->dev);
drivers/net/virtio_net.c static struct virtio_device_id id_table[] = { · line 2819
Quoted 9 lines
drivers/net/virtio_net.c +2
-1@@ -2796,7 +2812,8 @@ static struct virtio_device_id id_table[] = { VIRTIO_NET_F_CTRL_RX, VIRTIO_NET_F_CTRL_VLAN, \VIRTIO_NET_F_GUEST_ANNOUNCE ,VIRTIO_NET_F_MQ , \VIRTIO_NET_F_CTRL_MAC_ADDR , \ - VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS +VIRTIO_NET_F_MTU ,VIRTIO_NET_F_CTRL_GUEST_OFFLOADS , \ +VIRTIO_NET_F_SPEED_DUPLEX static unsigned int features[] = {VIRTNET_FEATURES ,
Still missing the update from virtnet_config_changed_work, and I think it's important to reflex host changes within guest when the feature bit has been acked.
Quoted 33 lines
include/uapi/linux/virtio_net.h +13
-0@@ -57,6 +57,8 @@* Steering */ #define VIRTIO_NET_F_CTRL_MAC_ADDR 23/* Set MAC address */ +#define VIRTIO_NET_F_SPEED_DUPLEX 63/* Device set linkspeed and duplex */ +#ifndef VIRTIO_NET_NO_LEGACY #define VIRTIO_NET_F_GSO 6/* Host handles pkts w/ any GSO type */ #endif /* VIRTIO_NET_NO_LEGACY */ @@ -76,6 +78,17 @@ struct virtio_net_config {__u16 max_virtqueue_pairs;/* Default maximum transmit unit advice */ __u16 mtu; +/* +* speed, in units of 1Mb. All values 0 to INT_MAX are legal. +* Any other value stands for unknown. +*/ +__u32 speed; +/* +* 0x00 - half duplex +* 0x01 - full duplex +* Any other value stands for unknown. +*/ +__u8 duplex; } __attribute__((packed));/* Signature · 1 line
2.6.1
Michael S. Tsirkin
maintainerRe: [PATCH net-next v3 1/3] virtio_net: propagate linkspeed/duplex settings from the hypervisor
In reply to Jason BaronQuoted 53 lines · Jason Baron
The ability to set speed and duplex for
virtio_netis useful in various scenarios as described here:16032be
virtio_net: add ethtool support for set and get of settingsHowever, it would be nice to be able to set this from the hypervisor, such that
virtio_netdoesn't require custom guest ethtool commands.Introduce a new feature flag,
VIRTIO_NET_F_SPEED_DUPLEX, which allows the hypervisor to export a linkspeed and duplex setting. The user can subsequently overwrite it later if desired via: 'ethtool -s'.Note that
VIRTIO_NET_F_SPEED_DUPLEXis defined as bit 63, the intention is that device feature bits are to grow down from bit 63, since the transports are starting from bit 24 and growing up.Signed-off-by Jason Baron <[email protected]>Cc "Michael S. Tsirkin" <[email protected]>Cc Jason Wang <[email protected]>--- drivers/net/virtio_net.c | 19 ++++++++++++++++++- include/uapi/linux/virtio_net.h | 13 +++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-)drivers/net/virtio_net.c +16
-0@@ -2146,6 +2146,22 @@ static void virtnet_config_changed_work(struct work_struct *work) vi->status = v; +if (virtio_has_feature (vi->vdev,VIRTIO_NET_F_SPEED_DUPLEX )) { +u32 speed; +u8 duplex; + + speed =virtio_cread32 (vi->vdev, + offsetof(struct virtio_net_config , + speed)); +if (ethtool_validate_speed (speed)) + vi->speed = speed; + duplex =virtio_cread8 (vi->vdev, + offsetof(struct virtio_net_config , + duplex)); +if (ethtool_validate_duplex (duplex)) + vi->duplex = duplex; + } +if (vi->status &VIRTIO_NET_S_LINK_UP ) {netif_carrier_on (vi->dev);netif_tx_wake_all_queues (vi->dev);
drivers/net/virtio_net.c static struct virtio_device_id id_table[] = { · line 2819
Quoted 9 lines
drivers/net/virtio_net.c +2
-1@@ -2796,7 +2812,8 @@ static struct virtio_device_id id_table[] = { VIRTIO_NET_F_CTRL_RX, VIRTIO_NET_F_CTRL_VLAN, \VIRTIO_NET_F_GUEST_ANNOUNCE ,VIRTIO_NET_F_MQ , \VIRTIO_NET_F_CTRL_MAC_ADDR , \ - VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS +VIRTIO_NET_F_MTU ,VIRTIO_NET_F_CTRL_GUEST_OFFLOADS , \ +VIRTIO_NET_F_SPEED_DUPLEX static unsigned int features[] = {VIRTNET_FEATURES ,
Still missing the update from virtnet_config_changed_work, and I think it's important to reflex host changes within guest when the feature bit has been acked.
Quoted 33 lines
include/uapi/linux/virtio_net.h +13
-0@@ -57,6 +57,8 @@* Steering */ #define VIRTIO_NET_F_CTRL_MAC_ADDR 23/* Set MAC address */ +#define VIRTIO_NET_F_SPEED_DUPLEX 63/* Device set linkspeed and duplex */ +#ifndef VIRTIO_NET_NO_LEGACY #define VIRTIO_NET_F_GSO 6/* Host handles pkts w/ any GSO type */ #endif /* VIRTIO_NET_NO_LEGACY */ @@ -76,6 +78,17 @@ struct virtio_net_config {__u16 max_virtqueue_pairs;/* Default maximum transmit unit advice */ __u16 mtu; +/* +* speed, in units of 1Mb. All values 0 to INT_MAX are legal. +* Any other value stands for unknown. +*/ +__u32 speed; +/* +* 0x00 - half duplex +* 0x01 - full duplex +* Any other value stands for unknown. +*/ +__u8 duplex; } __attribute__((packed));/* Signature · 1 line
2.6.1
Re: [PATCH net-next v3 1/3] virtio_net: propagate linkspeed/duplex settings from the hypervisor
Quoted 61 lines · Michael S. Tsirkin
Quoted 60 lines · Jason Baron
The ability to set speed and duplex for
virtio_netis useful in various scenarios as described here:16032be
virtio_net: add ethtool support for set and get of settingsHowever, it would be nice to be able to set this from the hypervisor, such that
virtio_netdoesn't require custom guest ethtool commands.Introduce a new feature flag,
VIRTIO_NET_F_SPEED_DUPLEX, which allows the hypervisor to export a linkspeed and duplex setting. The user can subsequently overwrite it later if desired via: 'ethtool -s'.Note that
VIRTIO_NET_F_SPEED_DUPLEXis defined as bit 63, the intention is that device feature bits are to grow down from bit 63, since the transports are starting from bit 24 and growing up.Signed-off-by Jason Baron <[email protected]>Cc "Michael S. Tsirkin" <[email protected]>Cc Jason Wang <[email protected]>--- drivers/net/virtio_net.c | 19 ++++++++++++++++++- include/uapi/linux/virtio_net.h | 13 +++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-)drivers/net/virtio_net.c +18
-1@@ -2146,6 +2146,22 @@ static void virtnet_config_changed_work(struct work_struct *work) vi->status = v; +if (virtio_has_feature (vi->vdev,VIRTIO_NET_F_SPEED_DUPLEX )) { +u32 speed; +u8 duplex; + + speed =virtio_cread32 (vi->vdev, + offsetof(struct virtio_net_config , + speed)); +if (ethtool_validate_speed (speed)) + vi->speed = speed; + duplex =virtio_cread8 (vi->vdev, + offsetof(struct virtio_net_config , + duplex)); +if (ethtool_validate_duplex (duplex)) + vi->duplex = duplex; + } +if (vi->status &VIRTIO_NET_S_LINK_UP ) {netif_carrier_on (vi->dev);netif_tx_wake_all_queues (vi->dev);@@ -2796,7 +2812,8 @@ static struct virtio_device_id id_table[] = {VIRTIO_NET_F_CTRL_RX ,VIRTIO_NET_F_CTRL_VLAN , \VIRTIO_NET_F_GUEST_ANNOUNCE ,VIRTIO_NET_F_MQ , \VIRTIO_NET_F_CTRL_MAC_ADDR , \ -VIRTIO_NET_F_MTU ,VIRTIO_NET_F_CTRL_GUEST_OFFLOADS +VIRTIO_NET_F_MTU ,VIRTIO_NET_F_CTRL_GUEST_OFFLOADS , \ +VIRTIO_NET_F_SPEED_DUPLEX
Quoted 7 lines
drivers/net/virtio_net.c static struct virtio_device_id id_table[] = { · line 2819
Quoted 2 lines
drivers/net/virtio_net.c +0
-0static unsigned int features[] = {VIRTNET_FEATURES ,Still missing the update from
virtnet_config_changed_work, and I think it's important to reflex host changes within guest when the feature bit has been acked.
I update vi->speed and vi->duplex in virtnet_config_changed_work(). And I tested using the 'set_link' on/off from the qemu monitor. Specifically, an 'off' sets the speed and link to 'unknown', and an 'on' returns the speed and link to the configured speed and duplex. So they are being updated dynamically now. What host changes are you referring to that are not reflected?
Thanks,
-Jason
Quoted 33 lines
Quoted 33 lines
include/uapi/linux/virtio_net.h +13
-0@@ -57,6 +57,8 @@* Steering */ #define VIRTIO_NET_F_CTRL_MAC_ADDR 23/* Set MAC address */ +#define VIRTIO_NET_F_SPEED_DUPLEX 63/* Device set linkspeed and duplex */ +#ifndef VIRTIO_NET_NO_LEGACY #define VIRTIO_NET_F_GSO 6/* Host handles pkts w/ any GSO type */ #endif /* VIRTIO_NET_NO_LEGACY */ @@ -76,6 +78,17 @@ struct virtio_net_config {__u16 max_virtqueue_pairs;/* Default maximum transmit unit advice */ __u16 mtu; +/* +* speed, in units of 1Mb. All values 0 to INT_MAX are legal. +* Any other value stands for unknown. +*/ +__u32 speed; +/* +* 0x00 - half duplex +* 0x01 - full duplex +* Any other value stands for unknown. +*/ +__u8 duplex; } __attribute__((packed));/* Signature · 1 line
2.6.1
Re: [PATCH net-next v3 1/3] virtio_net: propagate linkspeed/duplex settings from the hypervisor
In reply to Michael S. TsirkinQuoted 61 lines · Michael S. Tsirkin
Quoted 60 lines · Jason Baron
The ability to set speed and duplex for
virtio_netis useful in various scenarios as described here:16032be
virtio_net: add ethtool support for set and get of settingsHowever, it would be nice to be able to set this from the hypervisor, such that
virtio_netdoesn't require custom guest ethtool commands.Introduce a new feature flag,
VIRTIO_NET_F_SPEED_DUPLEX, which allows the hypervisor to export a linkspeed and duplex setting. The user can subsequently overwrite it later if desired via: 'ethtool -s'.Note that
VIRTIO_NET_F_SPEED_DUPLEXis defined as bit 63, the intention is that device feature bits are to grow down from bit 63, since the transports are starting from bit 24 and growing up.Signed-off-by Jason Baron <[email protected]>Cc "Michael S. Tsirkin" <[email protected]>Cc Jason Wang <[email protected]>--- drivers/net/virtio_net.c | 19 ++++++++++++++++++- include/uapi/linux/virtio_net.h | 13 +++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-)drivers/net/virtio_net.c +18
-1@@ -2146,6 +2146,22 @@ static void virtnet_config_changed_work(struct work_struct *work) vi->status = v; +if (virtio_has_feature (vi->vdev,VIRTIO_NET_F_SPEED_DUPLEX )) { +u32 speed; +u8 duplex; + + speed =virtio_cread32 (vi->vdev, + offsetof(struct virtio_net_config , + speed)); +if (ethtool_validate_speed (speed)) + vi->speed = speed; + duplex =virtio_cread8 (vi->vdev, + offsetof(struct virtio_net_config , + duplex)); +if (ethtool_validate_duplex (duplex)) + vi->duplex = duplex; + } +if (vi->status &VIRTIO_NET_S_LINK_UP ) {netif_carrier_on (vi->dev);netif_tx_wake_all_queues (vi->dev);@@ -2796,7 +2812,8 @@ static struct virtio_device_id id_table[] = {VIRTIO_NET_F_CTRL_RX ,VIRTIO_NET_F_CTRL_VLAN , \VIRTIO_NET_F_GUEST_ANNOUNCE ,VIRTIO_NET_F_MQ , \VIRTIO_NET_F_CTRL_MAC_ADDR , \ -VIRTIO_NET_F_MTU ,VIRTIO_NET_F_CTRL_GUEST_OFFLOADS +VIRTIO_NET_F_MTU ,VIRTIO_NET_F_CTRL_GUEST_OFFLOADS , \ +VIRTIO_NET_F_SPEED_DUPLEX
Quoted 7 lines
drivers/net/virtio_net.c static struct virtio_device_id id_table[] = { · line 2819
Quoted 2 lines
drivers/net/virtio_net.c +0
-0static unsigned int features[] = {VIRTNET_FEATURES ,Still missing the update from
virtnet_config_changed_work, and I think it's important to reflex host changes within guest when the feature bit has been acked.
I update vi->speed and vi->duplex in virtnet_config_changed_work(). And I tested using the 'set_link' on/off from the qemu monitor. Specifically, an 'off' sets the speed and link to 'unknown', and an 'on' returns the speed and link to the configured speed and duplex. So they are being updated dynamically now. What host changes are you referring to that are not reflected?
Thanks,
-Jason
Quoted 33 lines
Quoted 33 lines
include/uapi/linux/virtio_net.h +13
-0@@ -57,6 +57,8 @@* Steering */ #define VIRTIO_NET_F_CTRL_MAC_ADDR 23/* Set MAC address */ +#define VIRTIO_NET_F_SPEED_DUPLEX 63/* Device set linkspeed and duplex */ +#ifndef VIRTIO_NET_NO_LEGACY #define VIRTIO_NET_F_GSO 6/* Host handles pkts w/ any GSO type */ #endif /* VIRTIO_NET_NO_LEGACY */ @@ -76,6 +78,17 @@ struct virtio_net_config {__u16 max_virtqueue_pairs;/* Default maximum transmit unit advice */ __u16 mtu; +/* +* speed, in units of 1Mb. All values 0 to INT_MAX are legal. +* Any other value stands for unknown. +*/ +__u32 speed; +/* +* 0x00 - half duplex +* 0x01 - full duplex +* Any other value stands for unknown. +*/ +__u8 duplex; } __attribute__((packed));/* Signature · 1 line
2.6.1
Michael S. Tsirkin
maintainerRe: [PATCH net-next v3 1/3] virtio_net: propagate linkspeed/duplex settings from the hypervisor
Quoted 79 lines · Jason Baron
Quoted 61 lines · Michael S. Tsirkin
Quoted 60 lines · Jason Baron
The ability to set speed and duplex for
virtio_netis useful in various scenarios as described here:16032be
virtio_net: add ethtool support for set and get of settingsHowever, it would be nice to be able to set this from the hypervisor, such that
virtio_netdoesn't require custom guest ethtool commands.Introduce a new feature flag,
VIRTIO_NET_F_SPEED_DUPLEX, which allows the hypervisor to export a linkspeed and duplex setting. The user can subsequently overwrite it later if desired via: 'ethtool -s'.Note that
VIRTIO_NET_F_SPEED_DUPLEXis defined as bit 63, the intention is that device feature bits are to grow down from bit 63, since the transports are starting from bit 24 and growing up.Signed-off-by Jason Baron <[email protected]>Cc "Michael S. Tsirkin" <[email protected]>Cc Jason Wang <[email protected]>--- drivers/net/virtio_net.c | 19 ++++++++++++++++++- include/uapi/linux/virtio_net.h | 13 +++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-)drivers/net/virtio_net.c +18
-1@@ -2146,6 +2146,22 @@ static void virtnet_config_changed_work(struct work_struct *work) vi->status = v; +if (virtio_has_feature (vi->vdev,VIRTIO_NET_F_SPEED_DUPLEX )) { +u32 speed; +u8 duplex; + + speed =virtio_cread32 (vi->vdev, + offsetof(struct virtio_net_config , + speed)); +if (ethtool_validate_speed (speed)) + vi->speed = speed; + duplex =virtio_cread8 (vi->vdev, + offsetof(struct virtio_net_config , + duplex)); +if (ethtool_validate_duplex (duplex)) + vi->duplex = duplex; + } +if (vi->status &VIRTIO_NET_S_LINK_UP ) {netif_carrier_on (vi->dev);netif_tx_wake_all_queues (vi->dev);@@ -2796,7 +2812,8 @@ static struct virtio_device_id id_table[] = {VIRTIO_NET_F_CTRL_RX ,VIRTIO_NET_F_CTRL_VLAN , \VIRTIO_NET_F_GUEST_ANNOUNCE ,VIRTIO_NET_F_MQ , \VIRTIO_NET_F_CTRL_MAC_ADDR , \ -VIRTIO_NET_F_MTU ,VIRTIO_NET_F_CTRL_GUEST_OFFLOADS +VIRTIO_NET_F_MTU ,VIRTIO_NET_F_CTRL_GUEST_OFFLOADS , \ +VIRTIO_NET_F_SPEED_DUPLEX Quoted 7 lines
drivers/net/virtio_net.c static struct virtio_device_id id_table[] = { · line 2819
Quoted 2 lines
drivers/net/virtio_net.c +0
-0static unsigned int features[] = {VIRTNET_FEATURES ,Still missing the update from
virtnet_config_changed_work, and I think it's important to reflex host changes within guest when the feature bit has been acked.I update
vi->speedandvi->duplexinvirtnet_config_changed_work(). And I tested using the 'set_link' on/off from the qemu monitor. Specifically, an 'off' sets the speed and link to 'unknown', and an 'on' returns the speed and link to the configured speed and duplex. So they are being updated dynamically now. What host changes are you referring to that are not reflected?
Quoted 3 lines
Thanks,
-Jason
Ouch, I was reviewing an old version and replied to this one. Sorry, will re-read now.
Quoted 34 lines
Quoted 34 lines
Quoted 34 lines
include/uapi/linux/virtio_net.h +13
-0@@ -57,6 +57,8 @@* Steering */ #define VIRTIO_NET_F_CTRL_MAC_ADDR 23/* Set MAC address */ +#define VIRTIO_NET_F_SPEED_DUPLEX 63/* Device set linkspeed and duplex */ +#ifndef VIRTIO_NET_NO_LEGACY #define VIRTIO_NET_F_GSO 6/* Host handles pkts w/ any GSO type */ #endif /* VIRTIO_NET_NO_LEGACY */ @@ -76,6 +78,17 @@ struct virtio_net_config {__u16 max_virtqueue_pairs;/* Default maximum transmit unit advice */ __u16 mtu; +/* +* speed, in units of 1Mb. All values 0 to INT_MAX are legal. +* Any other value stands for unknown. +*/ +__u32 speed; +/* +* 0x00 - half duplex +* 0x01 - full duplex +* Any other value stands for unknown. +*/ +__u8 duplex; } __attribute__((packed));/* Signature · 1 line
2.6.1
Michael S. Tsirkin
maintainerRe: [PATCH net-next v3 1/3] virtio_net: propagate linkspeed/duplex settings from the hypervisor
In reply to Jason BaronQuoted 79 lines · Jason Baron
Quoted 61 lines · Michael S. Tsirkin
Quoted 60 lines · Jason Baron
The ability to set speed and duplex for
virtio_netis useful in various scenarios as described here:16032be
virtio_net: add ethtool support for set and get of settingsHowever, it would be nice to be able to set this from the hypervisor, such that
virtio_netdoesn't require custom guest ethtool commands.Introduce a new feature flag,
VIRTIO_NET_F_SPEED_DUPLEX, which allows the hypervisor to export a linkspeed and duplex setting. The user can subsequently overwrite it later if desired via: 'ethtool -s'.Note that
VIRTIO_NET_F_SPEED_DUPLEXis defined as bit 63, the intention is that device feature bits are to grow down from bit 63, since the transports are starting from bit 24 and growing up.Signed-off-by Jason Baron <[email protected]>Cc "Michael S. Tsirkin" <[email protected]>Cc Jason Wang <[email protected]>--- drivers/net/virtio_net.c | 19 ++++++++++++++++++- include/uapi/linux/virtio_net.h | 13 +++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-)drivers/net/virtio_net.c +18
-1@@ -2146,6 +2146,22 @@ static void virtnet_config_changed_work(struct work_struct *work) vi->status = v; +if (virtio_has_feature (vi->vdev,VIRTIO_NET_F_SPEED_DUPLEX )) { +u32 speed; +u8 duplex; + + speed =virtio_cread32 (vi->vdev, + offsetof(struct virtio_net_config , + speed)); +if (ethtool_validate_speed (speed)) + vi->speed = speed; + duplex =virtio_cread8 (vi->vdev, + offsetof(struct virtio_net_config , + duplex)); +if (ethtool_validate_duplex (duplex)) + vi->duplex = duplex; + } +if (vi->status &VIRTIO_NET_S_LINK_UP ) {netif_carrier_on (vi->dev);netif_tx_wake_all_queues (vi->dev);@@ -2796,7 +2812,8 @@ static struct virtio_device_id id_table[] = {VIRTIO_NET_F_CTRL_RX ,VIRTIO_NET_F_CTRL_VLAN , \VIRTIO_NET_F_GUEST_ANNOUNCE ,VIRTIO_NET_F_MQ , \VIRTIO_NET_F_CTRL_MAC_ADDR , \ -VIRTIO_NET_F_MTU ,VIRTIO_NET_F_CTRL_GUEST_OFFLOADS +VIRTIO_NET_F_MTU ,VIRTIO_NET_F_CTRL_GUEST_OFFLOADS , \ +VIRTIO_NET_F_SPEED_DUPLEX Quoted 7 lines
drivers/net/virtio_net.c static struct virtio_device_id id_table[] = { · line 2819
Quoted 2 lines
drivers/net/virtio_net.c +0
-0static unsigned int features[] = {VIRTNET_FEATURES ,Still missing the update from
virtnet_config_changed_work, and I think it's important to reflex host changes within guest when the feature bit has been acked.I update
vi->speedandvi->duplexinvirtnet_config_changed_work(). And I tested using the 'set_link' on/off from the qemu monitor. Specifically, an 'off' sets the speed and link to 'unknown', and an 'on' returns the speed and link to the configured speed and duplex. So they are being updated dynamically now. What host changes are you referring to that are not reflected?
Quoted 3 lines
Thanks,
-Jason
Ouch, I was reviewing an old version and replied to this one. Sorry, will re-read now.
Quoted 34 lines
Quoted 34 lines
Quoted 34 lines
include/uapi/linux/virtio_net.h +13
-0@@ -57,6 +57,8 @@* Steering */ #define VIRTIO_NET_F_CTRL_MAC_ADDR 23/* Set MAC address */ +#define VIRTIO_NET_F_SPEED_DUPLEX 63/* Device set linkspeed and duplex */ +#ifndef VIRTIO_NET_NO_LEGACY #define VIRTIO_NET_F_GSO 6/* Host handles pkts w/ any GSO type */ #endif /* VIRTIO_NET_NO_LEGACY */ @@ -76,6 +78,17 @@ struct virtio_net_config {__u16 max_virtqueue_pairs;/* Default maximum transmit unit advice */ __u16 mtu; +/* +* speed, in units of 1Mb. All values 0 to INT_MAX are legal. +* Any other value stands for unknown. +*/ +__u32 speed; +/* +* 0x00 - half duplex +* 0x01 - full duplex +* Any other value stands for unknown. +*/ +__u8 duplex; } __attribute__((packed));/* Signature · 1 line
2.6.1
Michael S. Tsirkin
maintainerRe: [PATCH net-next v3 1/3] virtio_net: propagate linkspeed/duplex settings from the hypervisor
In reply to Jason BaronQuoted 30 lines · Jason Baron
The ability to set speed and duplex for
virtio_netis useful in various scenarios as described here:16032be
virtio_net: add ethtool support for set and get of settingsHowever, it would be nice to be able to set this from the hypervisor, such that
virtio_netdoesn't require custom guest ethtool commands.Introduce a new feature flag,
VIRTIO_NET_F_SPEED_DUPLEX, which allows the hypervisor to export a linkspeed and duplex setting. The user can subsequently overwrite it later if desired via: 'ethtool -s'.Note that
VIRTIO_NET_F_SPEED_DUPLEXis defined as bit 63, the intention is that device feature bits are to grow down from bit 63, since the transports are starting from bit 24 and growing up.Signed-off-by Jason Baron <[email protected]>Cc "Michael S. Tsirkin" <[email protected]>Cc Jason Wang <[email protected]>--- drivers/net/virtio_net.c | 19 ++++++++++++++++++- include/uapi/linux/virtio_net.h | 13 +++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-)drivers/net/virtio_net.c +0
-0
drivers/net/virtio_net.c static void virtnet_config_changed_work(struct work_struct *work) · line 2149
Quoted 4 lines
drivers/net/virtio_net.c +1
-0@@ -2146,6 +2146,22 @@ static void virtnet_config_changed_work(struct work_struct *work) vi->status = v; +if (virtio_has_feature (vi->vdev,VIRTIO_NET_F_SPEED_DUPLEX )) {
BTW we can avoid this read for when link goes down. Not a big deal but still.
Quoted 8 lines
drivers/net/virtio_net.c +8
-0+u32 speed; +u8 duplex; + + speed =virtio_cread32 (vi->vdev, + offsetof(struct virtio_net_config , + speed)); +if (ethtool_validate_speed (speed)) + vi->speed = speed;
drivers/net/virtio_net.c static void virtnet_config_changed_work(struct work_struct *work) · line 2167
Quoted 10 lines
drivers/net/virtio_net.c +7
-0+ duplex =virtio_cread8 (vi->vdev, + offsetof(struct virtio_net_config , + duplex)); +if (ethtool_validate_duplex (duplex)) + vi->duplex = duplex; + } +if (vi->status &VIRTIO_NET_S_LINK_UP ) {netif_carrier_on (vi->dev);netif_tx_wake_all_queues (vi->dev);
OK so this handles the case when VIRTIO_NET_F_STATUS is set, but when it's clear we need to call this from virtnet_probe.
I propose moving this chunk to a function and calling from two places.
Quoted 43 lines
drivers/net/virtio_net.c +2
-1@@ -2796,7 +2812,8 @@ static struct virtio_device_id id_table[] = { VIRTIO_NET_F_CTRL_RX, VIRTIO_NET_F_CTRL_VLAN, \VIRTIO_NET_F_GUEST_ANNOUNCE ,VIRTIO_NET_F_MQ , \VIRTIO_NET_F_CTRL_MAC_ADDR , \ - VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS +VIRTIO_NET_F_MTU ,VIRTIO_NET_F_CTRL_GUEST_OFFLOADS , \ +VIRTIO_NET_F_SPEED_DUPLEX static unsigned int features[] = {VIRTNET_FEATURES ,include/uapi/linux/virtio_net.h +13
-0@@ -57,6 +57,8 @@* Steering */ #define VIRTIO_NET_F_CTRL_MAC_ADDR 23/* Set MAC address */ +#define VIRTIO_NET_F_SPEED_DUPLEX 63/* Device set linkspeed and duplex */ +#ifndef VIRTIO_NET_NO_LEGACY #define VIRTIO_NET_F_GSO 6/* Host handles pkts w/ any GSO type */ #endif /* VIRTIO_NET_NO_LEGACY */ @@ -76,6 +78,17 @@ struct virtio_net_config {__u16 max_virtqueue_pairs;/* Default maximum transmit unit advice */ __u16 mtu; +/* +* speed, in units of 1Mb. All values 0 to INT_MAX are legal. +* Any other value stands for unknown. +*/ +__u32 speed; +/* +* 0x00 - half duplex +* 0x01 - full duplex +* Any other value stands for unknown. +*/ +__u8 duplex; } __attribute__((packed));/* Signature · 1 line
2.6.1
Michael S. Tsirkin
maintainerRe: [PATCH net-next v3 1/3] virtio_net: propagate linkspeed/duplex settings from the hypervisor
In reply to Jason BaronQuoted 30 lines · Jason Baron
The ability to set speed and duplex for
virtio_netis useful in various scenarios as described here:16032be
virtio_net: add ethtool support for set and get of settingsHowever, it would be nice to be able to set this from the hypervisor, such that
virtio_netdoesn't require custom guest ethtool commands.Introduce a new feature flag,
VIRTIO_NET_F_SPEED_DUPLEX, which allows the hypervisor to export a linkspeed and duplex setting. The user can subsequently overwrite it later if desired via: 'ethtool -s'.Note that
VIRTIO_NET_F_SPEED_DUPLEXis defined as bit 63, the intention is that device feature bits are to grow down from bit 63, since the transports are starting from bit 24 and growing up.Signed-off-by Jason Baron <[email protected]>Cc "Michael S. Tsirkin" <[email protected]>Cc Jason Wang <[email protected]>--- drivers/net/virtio_net.c | 19 ++++++++++++++++++- include/uapi/linux/virtio_net.h | 13 +++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-)drivers/net/virtio_net.c +0
-0
drivers/net/virtio_net.c static void virtnet_config_changed_work(struct work_struct *work) · line 2149
Quoted 4 lines
drivers/net/virtio_net.c +1
-0@@ -2146,6 +2146,22 @@ static void virtnet_config_changed_work(struct work_struct *work) vi->status = v; +if (virtio_has_feature (vi->vdev,VIRTIO_NET_F_SPEED_DUPLEX )) {
BTW we can avoid this read for when link goes down. Not a big deal but still.
Quoted 8 lines
drivers/net/virtio_net.c +8
-0+u32 speed; +u8 duplex; + + speed =virtio_cread32 (vi->vdev, + offsetof(struct virtio_net_config , + speed)); +if (ethtool_validate_speed (speed)) + vi->speed = speed;
drivers/net/virtio_net.c static void virtnet_config_changed_work(struct work_struct *work) · line 2167
Quoted 10 lines
drivers/net/virtio_net.c +7
-0+ duplex =virtio_cread8 (vi->vdev, + offsetof(struct virtio_net_config , + duplex)); +if (ethtool_validate_duplex (duplex)) + vi->duplex = duplex; + } +if (vi->status &VIRTIO_NET_S_LINK_UP ) {netif_carrier_on (vi->dev);netif_tx_wake_all_queues (vi->dev);
OK so this handles the case when VIRTIO_NET_F_STATUS is set, but when it's clear we need to call this from virtnet_probe.
I propose moving this chunk to a function and calling from two places.
Quoted 43 lines
drivers/net/virtio_net.c +2
-1@@ -2796,7 +2812,8 @@ static struct virtio_device_id id_table[] = { VIRTIO_NET_F_CTRL_RX, VIRTIO_NET_F_CTRL_VLAN, \VIRTIO_NET_F_GUEST_ANNOUNCE ,VIRTIO_NET_F_MQ , \VIRTIO_NET_F_CTRL_MAC_ADDR , \ - VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS +VIRTIO_NET_F_MTU ,VIRTIO_NET_F_CTRL_GUEST_OFFLOADS , \ +VIRTIO_NET_F_SPEED_DUPLEX static unsigned int features[] = {VIRTNET_FEATURES ,include/uapi/linux/virtio_net.h +13
-0@@ -57,6 +57,8 @@* Steering */ #define VIRTIO_NET_F_CTRL_MAC_ADDR 23/* Set MAC address */ +#define VIRTIO_NET_F_SPEED_DUPLEX 63/* Device set linkspeed and duplex */ +#ifndef VIRTIO_NET_NO_LEGACY #define VIRTIO_NET_F_GSO 6/* Host handles pkts w/ any GSO type */ #endif /* VIRTIO_NET_NO_LEGACY */ @@ -76,6 +78,17 @@ struct virtio_net_config {__u16 max_virtqueue_pairs;/* Default maximum transmit unit advice */ __u16 mtu; +/* +* speed, in units of 1Mb. All values 0 to INT_MAX are legal. +* Any other value stands for unknown. +*/ +__u32 speed; +/* +* 0x00 - half duplex +* 0x01 - full duplex +* Any other value stands for unknown. +*/ +__u8 duplex; } __attribute__((packed));/* Signature · 1 line
2.6.1
Re: [PATCH net-next v3 1/3] virtio_net: propagate linkspeed/duplex settings from the hypervisor
Quoted 32 lines · Michael S. Tsirkin
Quoted 31 lines · Jason Baron
The ability to set speed and duplex for
virtio_netis useful in various scenarios as described here:16032be
virtio_net: add ethtool support for set and get of settingsHowever, it would be nice to be able to set this from the hypervisor, such that
virtio_netdoesn't require custom guest ethtool commands.Introduce a new feature flag,
VIRTIO_NET_F_SPEED_DUPLEX, which allows the hypervisor to export a linkspeed and duplex setting. The user can subsequently overwrite it later if desired via: 'ethtool -s'.Note that
VIRTIO_NET_F_SPEED_DUPLEXis defined as bit 63, the intention is that device feature bits are to grow down from bit 63, since the transports are starting from bit 24 and growing up.Signed-off-by Jason Baron <[email protected]>Cc "Michael S. Tsirkin" <[email protected]>Cc Jason Wang <[email protected]>--- drivers/net/virtio_net.c | 19 ++++++++++++++++++- include/uapi/linux/virtio_net.h | 13 +++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-)drivers/net/virtio_net.c +0
-0@@ -2146,6 +2146,22 @@ static void virtnet_config_changed_work(struct work_struct *work)
Quoted 6 lines
drivers/net/virtio_net.c static void virtnet_config_changed_work(struct work_struct *work) · line 2149
Quoted 3 lines
drivers/net/virtio_net.c +1
-0vi->status = v; +if (virtio_has_feature (vi->vdev,VIRTIO_NET_F_SPEED_DUPLEX )) {BTW we can avoid this read for when link goes down. Not a big deal but still.
So you are saying that we can just set vi->speed and vi->duplex to 'unknown' when the link goes down and not check for the presence of VIRTIO_NET_F_SPEED_DUPLEX?
If so, that could over-write what the user may have configured in the guest via 'ethtool -s' when the link goes down, so that would be a change in behavior, but perhaps that is ok?
I think I would prefer to have the link down event still check for VIRTIO_NET_F_SPEED_DUPLEX before changing speed/duplex. That way we still have 2 modes for updating the fields:
1) completely guest controlled. Same as we have now and host does not change any values and does not set VIRTIO_NET_F_SPEED_DUPLEX flag (hence don't remove above check).
2) if speed or duplex or speed is set in the qemu command line, then set the VIRTIO_NET_F_SPEED_DUPLEX and have host control the settings of speed/duplex (with ability of guest to over-write if it wanted to).
Quoted 23 lines
Quoted 9 lines
drivers/net/virtio_net.c +8
-0+u32 speed; +u8 duplex; + + speed =virtio_cread32 (vi->vdev, + offsetof(struct virtio_net_config , + speed)); +if (ethtool_validate_speed (speed)) + vi->speed = speed;drivers/net/virtio_net.c static void virtnet_config_changed_work(struct work_struct *work) · line 2167
Quoted 10 lines
drivers/net/virtio_net.c +7
-0+ duplex =virtio_cread8 (vi->vdev, + offsetof(struct virtio_net_config , + duplex)); +if (ethtool_validate_duplex (duplex)) + vi->duplex = duplex; + } +if (vi->status &VIRTIO_NET_S_LINK_UP ) {netif_carrier_on (vi->dev);netif_tx_wake_all_queues (vi->dev);OK so this handles the case when
VIRTIO_NET_F_STATUSis set, but when it's clear we need to call this fromvirtnet_probe.
Quoted 2 lines
I propose moving this chunk to a function and calling from two places.
good point. will update.
Thanks,
-Jason
Quoted 44 lines
Quoted 44 lines
drivers/net/virtio_net.c +2
-1@@ -2796,7 +2812,8 @@ static struct virtio_device_id id_table[] = { VIRTIO_NET_F_CTRL_RX, VIRTIO_NET_F_CTRL_VLAN, \VIRTIO_NET_F_GUEST_ANNOUNCE ,VIRTIO_NET_F_MQ , \VIRTIO_NET_F_CTRL_MAC_ADDR , \ - VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS +VIRTIO_NET_F_MTU ,VIRTIO_NET_F_CTRL_GUEST_OFFLOADS , \ +VIRTIO_NET_F_SPEED_DUPLEX static unsigned int features[] = {VIRTNET_FEATURES ,include/uapi/linux/virtio_net.h +13
-0@@ -57,6 +57,8 @@* Steering */ #define VIRTIO_NET_F_CTRL_MAC_ADDR 23/* Set MAC address */ +#define VIRTIO_NET_F_SPEED_DUPLEX 63/* Device set linkspeed and duplex */ +#ifndef VIRTIO_NET_NO_LEGACY #define VIRTIO_NET_F_GSO 6/* Host handles pkts w/ any GSO type */ #endif /* VIRTIO_NET_NO_LEGACY */ @@ -76,6 +78,17 @@ struct virtio_net_config {__u16 max_virtqueue_pairs;/* Default maximum transmit unit advice */ __u16 mtu; +/* +* speed, in units of 1Mb. All values 0 to INT_MAX are legal. +* Any other value stands for unknown. +*/ +__u32 speed; +/* +* 0x00 - half duplex +* 0x01 - full duplex +* Any other value stands for unknown. +*/ +__u8 duplex; } __attribute__((packed));/* Signature · 1 line
2.6.1
Re: [PATCH net-next v3 1/3] virtio_net: propagate linkspeed/duplex settings from the hypervisor
In reply to Michael S. TsirkinQuoted 32 lines · Michael S. Tsirkin
Quoted 31 lines · Jason Baron
The ability to set speed and duplex for
virtio_netis useful in various scenarios as described here:16032be
virtio_net: add ethtool support for set and get of settingsHowever, it would be nice to be able to set this from the hypervisor, such that
virtio_netdoesn't require custom guest ethtool commands.Introduce a new feature flag,
VIRTIO_NET_F_SPEED_DUPLEX, which allows the hypervisor to export a linkspeed and duplex setting. The user can subsequently overwrite it later if desired via: 'ethtool -s'.Note that
VIRTIO_NET_F_SPEED_DUPLEXis defined as bit 63, the intention is that device feature bits are to grow down from bit 63, since the transports are starting from bit 24 and growing up.Signed-off-by Jason Baron <[email protected]>Cc "Michael S. Tsirkin" <[email protected]>Cc Jason Wang <[email protected]>--- drivers/net/virtio_net.c | 19 ++++++++++++++++++- include/uapi/linux/virtio_net.h | 13 +++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-)drivers/net/virtio_net.c +0
-0@@ -2146,6 +2146,22 @@ static void virtnet_config_changed_work(struct work_struct *work)
Quoted 6 lines
drivers/net/virtio_net.c static void virtnet_config_changed_work(struct work_struct *work) · line 2149
Quoted 3 lines
drivers/net/virtio_net.c +1
-0vi->status = v; +if (virtio_has_feature (vi->vdev,VIRTIO_NET_F_SPEED_DUPLEX )) {BTW we can avoid this read for when link goes down. Not a big deal but still.
So you are saying that we can just set vi->speed and vi->duplex to 'unknown' when the link goes down and not check for the presence of VIRTIO_NET_F_SPEED_DUPLEX?
If so, that could over-write what the user may have configured in the guest via 'ethtool -s' when the link goes down, so that would be a change in behavior, but perhaps that is ok?
I think I would prefer to have the link down event still check for VIRTIO_NET_F_SPEED_DUPLEX before changing speed/duplex. That way we still have 2 modes for updating the fields:
1) completely guest controlled. Same as we have now and host does not change any values and does not set VIRTIO_NET_F_SPEED_DUPLEX flag (hence don't remove above check).
2) if speed or duplex or speed is set in the qemu command line, then set the VIRTIO_NET_F_SPEED_DUPLEX and have host control the settings of speed/duplex (with ability of guest to over-write if it wanted to).
Quoted 23 lines
Quoted 9 lines
drivers/net/virtio_net.c +8
-0+u32 speed; +u8 duplex; + + speed =virtio_cread32 (vi->vdev, + offsetof(struct virtio_net_config , + speed)); +if (ethtool_validate_speed (speed)) + vi->speed = speed;drivers/net/virtio_net.c static void virtnet_config_changed_work(struct work_struct *work) · line 2167
Quoted 10 lines
drivers/net/virtio_net.c +7
-0+ duplex =virtio_cread8 (vi->vdev, + offsetof(struct virtio_net_config , + duplex)); +if (ethtool_validate_duplex (duplex)) + vi->duplex = duplex; + } +if (vi->status &VIRTIO_NET_S_LINK_UP ) {netif_carrier_on (vi->dev);netif_tx_wake_all_queues (vi->dev);OK so this handles the case when
VIRTIO_NET_F_STATUSis set, but when it's clear we need to call this fromvirtnet_probe.
Quoted 2 lines
I propose moving this chunk to a function and calling from two places.
good point. will update.
Thanks,
-Jason
Quoted 44 lines
Quoted 44 lines
drivers/net/virtio_net.c +2
-1@@ -2796,7 +2812,8 @@ static struct virtio_device_id id_table[] = { VIRTIO_NET_F_CTRL_RX, VIRTIO_NET_F_CTRL_VLAN, \VIRTIO_NET_F_GUEST_ANNOUNCE ,VIRTIO_NET_F_MQ , \VIRTIO_NET_F_CTRL_MAC_ADDR , \ - VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS +VIRTIO_NET_F_MTU ,VIRTIO_NET_F_CTRL_GUEST_OFFLOADS , \ +VIRTIO_NET_F_SPEED_DUPLEX static unsigned int features[] = {VIRTNET_FEATURES ,include/uapi/linux/virtio_net.h +13
-0@@ -57,6 +57,8 @@* Steering */ #define VIRTIO_NET_F_CTRL_MAC_ADDR 23/* Set MAC address */ +#define VIRTIO_NET_F_SPEED_DUPLEX 63/* Device set linkspeed and duplex */ +#ifndef VIRTIO_NET_NO_LEGACY #define VIRTIO_NET_F_GSO 6/* Host handles pkts w/ any GSO type */ #endif /* VIRTIO_NET_NO_LEGACY */ @@ -76,6 +78,17 @@ struct virtio_net_config {__u16 max_virtqueue_pairs;/* Default maximum transmit unit advice */ __u16 mtu; +/* +* speed, in units of 1Mb. All values 0 to INT_MAX are legal. +* Any other value stands for unknown. +*/ +__u32 speed; +/* +* 0x00 - half duplex +* 0x01 - full duplex +* Any other value stands for unknown. +*/ +__u8 duplex; } __attribute__((packed));/* Signature · 1 line
2.6.1
Michael S. Tsirkin
maintainerRe: [PATCH net-next v3 1/3] virtio_net: propagate linkspeed/duplex settings from the hypervisor
Quoted 42 lines · Jason Baron
Quoted 32 lines · Michael S. Tsirkin
Quoted 31 lines · Jason Baron
The ability to set speed and duplex for
virtio_netis useful in various scenarios as described here:16032be
virtio_net: add ethtool support for set and get of settingsHowever, it would be nice to be able to set this from the hypervisor, such that
virtio_netdoesn't require custom guest ethtool commands.Introduce a new feature flag,
VIRTIO_NET_F_SPEED_DUPLEX, which allows the hypervisor to export a linkspeed and duplex setting. The user can subsequently overwrite it later if desired via: 'ethtool -s'.Note that
VIRTIO_NET_F_SPEED_DUPLEXis defined as bit 63, the intention is that device feature bits are to grow down from bit 63, since the transports are starting from bit 24 and growing up.Signed-off-by Jason Baron <[email protected]>Cc "Michael S. Tsirkin" <[email protected]>Cc Jason Wang <[email protected]>--- drivers/net/virtio_net.c | 19 ++++++++++++++++++- include/uapi/linux/virtio_net.h | 13 +++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-)drivers/net/virtio_net.c +0
-0@@ -2146,6 +2146,22 @@ static void virtnet_config_changed_work(struct work_struct *work)Quoted 6 lines
drivers/net/virtio_net.c static void virtnet_config_changed_work(struct work_struct *work) · line 2149
Quoted 3 lines
drivers/net/virtio_net.c +1
-0vi->status = v; +if (virtio_has_feature (vi->vdev,VIRTIO_NET_F_SPEED_DUPLEX )) {BTW we can avoid this read for when link goes down. Not a big deal but still.
Quoted 7 lines
So you are saying that we can just set
vi->speedandvi->duplexto 'unknown' when the link goes down and not check for the presence ofVIRTIO_NET_F_SPEED_DUPLEX?If so, that could over-write what the user may have configured in the guest via 'ethtool -s' when the link goes down, so that would be a change in behavior, but perhaps that is ok?
No - what I am saying is that your patch overwrites the values set by user when link goes down.
I suggest limiting this call to when
if (vi->status & VIRTIO_NET_S_LINK_UP)
and then the values are overwritten when link goes up which seems closer to what a user might expect.
Quoted 90 lines
I think I would prefer to have the link down event still check for
VIRTIO_NET_F_SPEED_DUPLEXbefore changing speed/duplex. That way we still have 2 modes for updating the fields:1) completely guest controlled. Same as we have now and host does not change any values and does not set
VIRTIO_NET_F_SPEED_DUPLEXflag (hence don't remove above check).2) if speed or duplex or speed is set in the qemu command line, then set the
VIRTIO_NET_F_SPEED_DUPLEXand have host control the settings of speed/duplex (with ability of guest to over-write if it wanted to).Quoted 23 lines
Quoted 9 lines
drivers/net/virtio_net.c +8
-0+u32 speed; +u8 duplex; + + speed =virtio_cread32 (vi->vdev, + offsetof(struct virtio_net_config , + speed)); +if (ethtool_validate_speed (speed)) + vi->speed = speed;drivers/net/virtio_net.c static void virtnet_config_changed_work(struct work_struct *work) · line 2167
Quoted 10 lines
drivers/net/virtio_net.c +7
-0+ duplex =virtio_cread8 (vi->vdev, + offsetof(struct virtio_net_config , + duplex)); +if (ethtool_validate_duplex (duplex)) + vi->duplex = duplex; + } +if (vi->status &VIRTIO_NET_S_LINK_UP ) {netif_carrier_on (vi->dev);netif_tx_wake_all_queues (vi->dev);OK so this handles the case when
VIRTIO_NET_F_STATUSis set, but when it's clear we need to call this fromvirtnet_probe.Quoted 2 lines
I propose moving this chunk to a function and calling from two places.
good point. will update.
Thanks,
-Jason
Quoted 44 lines
Quoted 44 lines
drivers/net/virtio_net.c +2
-1@@ -2796,7 +2812,8 @@ static struct virtio_device_id id_table[] = { VIRTIO_NET_F_CTRL_RX, VIRTIO_NET_F_CTRL_VLAN, \VIRTIO_NET_F_GUEST_ANNOUNCE ,VIRTIO_NET_F_MQ , \VIRTIO_NET_F_CTRL_MAC_ADDR , \ - VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS +VIRTIO_NET_F_MTU ,VIRTIO_NET_F_CTRL_GUEST_OFFLOADS , \ +VIRTIO_NET_F_SPEED_DUPLEX static unsigned int features[] = {VIRTNET_FEATURES ,include/uapi/linux/virtio_net.h +13
-0@@ -57,6 +57,8 @@* Steering */ #define VIRTIO_NET_F_CTRL_MAC_ADDR 23/* Set MAC address */ +#define VIRTIO_NET_F_SPEED_DUPLEX 63/* Device set linkspeed and duplex */ +#ifndef VIRTIO_NET_NO_LEGACY #define VIRTIO_NET_F_GSO 6/* Host handles pkts w/ any GSO type */ #endif /* VIRTIO_NET_NO_LEGACY */ @@ -76,6 +78,17 @@ struct virtio_net_config {__u16 max_virtqueue_pairs;/* Default maximum transmit unit advice */ __u16 mtu; +/* +* speed, in units of 1Mb. All values 0 to INT_MAX are legal. +* Any other value stands for unknown. +*/ +__u32 speed; +/* +* 0x00 - half duplex +* 0x01 - full duplex +* Any other value stands for unknown. +*/ +__u8 duplex; } __attribute__((packed));/* Signature · 1 line
2.6.1
Michael S. Tsirkin
maintainerRe: [PATCH net-next v3 1/3] virtio_net: propagate linkspeed/duplex settings from the hypervisor
Quoted 54 lines · Michael S. Tsirkin
Quoted 42 lines · Jason Baron
Quoted 32 lines · Michael S. Tsirkin
Quoted 31 lines · Jason Baron
The ability to set speed and duplex for
virtio_netis useful in various scenarios as described here:16032be
virtio_net: add ethtool support for set and get of settingsHowever, it would be nice to be able to set this from the hypervisor, such that
virtio_netdoesn't require custom guest ethtool commands.Introduce a new feature flag,
VIRTIO_NET_F_SPEED_DUPLEX, which allows the hypervisor to export a linkspeed and duplex setting. The user can subsequently overwrite it later if desired via: 'ethtool -s'.Note that
VIRTIO_NET_F_SPEED_DUPLEXis defined as bit 63, the intention is that device feature bits are to grow down from bit 63, since the transports are starting from bit 24 and growing up.Signed-off-by Jason Baron <[email protected]>Cc "Michael S. Tsirkin" <[email protected]>Cc Jason Wang <[email protected]>--- drivers/net/virtio_net.c | 19 ++++++++++++++++++- include/uapi/linux/virtio_net.h | 13 +++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-)drivers/net/virtio_net.c +0
-0@@ -2146,6 +2146,22 @@ static void virtnet_config_changed_work(struct work_struct *work)Quoted 6 lines
drivers/net/virtio_net.c static void virtnet_config_changed_work(struct work_struct *work) · line 2149
Quoted 3 lines
drivers/net/virtio_net.c +1
-0vi->status = v; +if (virtio_has_feature (vi->vdev,VIRTIO_NET_F_SPEED_DUPLEX )) {BTW we can avoid this read for when link goes down. Not a big deal but still.
Quoted 7 lines
So you are saying that we can just set
vi->speedandvi->duplexto 'unknown' when the link goes down and not check for the presence ofVIRTIO_NET_F_SPEED_DUPLEX?If so, that could over-write what the user may have configured in the guest via 'ethtool -s' when the link goes down, so that would be a change in behavior, but perhaps that is ok?
No - what I am saying is that your patch overwrites the values set by user when link goes down.
Quoted 19 lines
I suggest limiting this call to when
if (
vi->status&VIRTIO_NET_S_LINK_UP)and then the values are overwritten when link goes up which seems closer to what a user might expect.
Quoted 12 lines
I think I would prefer to have the link down event still check for
VIRTIO_NET_F_SPEED_DUPLEXbefore changing speed/duplex. That way we still have 2 modes for updating the fields:1) completely guest controlled. Same as we have now and host does not change any values and does not set
VIRTIO_NET_F_SPEED_DUPLEXflag (hence don't remove above check).2) if speed or duplex or speed is set in the qemu command line, then set the
VIRTIO_NET_F_SPEED_DUPLEXand have host control the settings of speed/duplex (with ability of guest to over-write if it wanted to).
I agree - I don't see a reason to touch the speed/duplex values when VIRTIO_NET_F_SPEED_DUPLEX has not been negotiated.
Quoted 78 lines
Quoted 78 lines
Quoted 25 lines
Quoted 11 lines
drivers/net/virtio_net.c +8
-0+u32 speed; +u8 duplex; + + speed =virtio_cread32 (vi->vdev, + offsetof(struct virtio_net_config , + speed)); +if (ethtool_validate_speed (speed)) + vi->speed = speed;drivers/net/virtio_net.c static void virtnet_config_changed_work(struct work_struct *work) · line 2167
Quoted 10 lines
drivers/net/virtio_net.c +7
-0+ duplex =virtio_cread8 (vi->vdev, + offsetof(struct virtio_net_config , + duplex)); +if (ethtool_validate_duplex (duplex)) + vi->duplex = duplex; + } +if (vi->status &VIRTIO_NET_S_LINK_UP ) {netif_carrier_on (vi->dev);netif_tx_wake_all_queues (vi->dev);OK so this handles the case when
VIRTIO_NET_F_STATUSis set, but when it's clear we need to call this fromvirtnet_probe.Quoted 2 lines
I propose moving this chunk to a function and calling from two places.
good point. will update.
Thanks,
-Jason
Quoted 44 lines
Quoted 44 lines
drivers/net/virtio_net.c +2
-1@@ -2796,7 +2812,8 @@ static struct virtio_device_id id_table[] = { VIRTIO_NET_F_CTRL_RX, VIRTIO_NET_F_CTRL_VLAN, \VIRTIO_NET_F_GUEST_ANNOUNCE ,VIRTIO_NET_F_MQ , \VIRTIO_NET_F_CTRL_MAC_ADDR , \ - VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS +VIRTIO_NET_F_MTU ,VIRTIO_NET_F_CTRL_GUEST_OFFLOADS , \ +VIRTIO_NET_F_SPEED_DUPLEX static unsigned int features[] = {VIRTNET_FEATURES ,include/uapi/linux/virtio_net.h +13
-0@@ -57,6 +57,8 @@* Steering */ #define VIRTIO_NET_F_CTRL_MAC_ADDR 23/* Set MAC address */ +#define VIRTIO_NET_F_SPEED_DUPLEX 63/* Device set linkspeed and duplex */ +#ifndef VIRTIO_NET_NO_LEGACY #define VIRTIO_NET_F_GSO 6/* Host handles pkts w/ any GSO type */ #endif /* VIRTIO_NET_NO_LEGACY */ @@ -76,6 +78,17 @@ struct virtio_net_config {__u16 max_virtqueue_pairs;/* Default maximum transmit unit advice */ __u16 mtu; +/* +* speed, in units of 1Mb. All values 0 to INT_MAX are legal. +* Any other value stands for unknown. +*/ +__u32 speed; +/* +* 0x00 - half duplex +* 0x01 - full duplex +* Any other value stands for unknown. +*/ +__u8 duplex; } __attribute__((packed));/* Signature · 1 line
2.6.1
Michael S. Tsirkin
maintainerRe: [PATCH net-next v3 1/3] virtio_net: propagate linkspeed/duplex settings from the hypervisor
In reply to Michael S. TsirkinQuoted 54 lines · Michael S. Tsirkin
Quoted 42 lines · Jason Baron
Quoted 32 lines · Michael S. Tsirkin
Quoted 31 lines · Jason Baron
The ability to set speed and duplex for
virtio_netis useful in various scenarios as described here:16032be
virtio_net: add ethtool support for set and get of settingsHowever, it would be nice to be able to set this from the hypervisor, such that
virtio_netdoesn't require custom guest ethtool commands.Introduce a new feature flag,
VIRTIO_NET_F_SPEED_DUPLEX, which allows the hypervisor to export a linkspeed and duplex setting. The user can subsequently overwrite it later if desired via: 'ethtool -s'.Note that
VIRTIO_NET_F_SPEED_DUPLEXis defined as bit 63, the intention is that device feature bits are to grow down from bit 63, since the transports are starting from bit 24 and growing up.Signed-off-by Jason Baron <[email protected]>Cc "Michael S. Tsirkin" <[email protected]>Cc Jason Wang <[email protected]>--- drivers/net/virtio_net.c | 19 ++++++++++++++++++- include/uapi/linux/virtio_net.h | 13 +++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-)drivers/net/virtio_net.c +0
-0@@ -2146,6 +2146,22 @@ static void virtnet_config_changed_work(struct work_struct *work)Quoted 6 lines
drivers/net/virtio_net.c static void virtnet_config_changed_work(struct work_struct *work) · line 2149
Quoted 3 lines
drivers/net/virtio_net.c +1
-0vi->status = v; +if (virtio_has_feature (vi->vdev,VIRTIO_NET_F_SPEED_DUPLEX )) {BTW we can avoid this read for when link goes down. Not a big deal but still.
Quoted 7 lines
So you are saying that we can just set
vi->speedandvi->duplexto 'unknown' when the link goes down and not check for the presence ofVIRTIO_NET_F_SPEED_DUPLEX?If so, that could over-write what the user may have configured in the guest via 'ethtool -s' when the link goes down, so that would be a change in behavior, but perhaps that is ok?
No - what I am saying is that your patch overwrites the values set by user when link goes down.
Quoted 19 lines
I suggest limiting this call to when
if (
vi->status&VIRTIO_NET_S_LINK_UP)and then the values are overwritten when link goes up which seems closer to what a user might expect.
Quoted 12 lines
I think I would prefer to have the link down event still check for
VIRTIO_NET_F_SPEED_DUPLEXbefore changing speed/duplex. That way we still have 2 modes for updating the fields:1) completely guest controlled. Same as we have now and host does not change any values and does not set
VIRTIO_NET_F_SPEED_DUPLEXflag (hence don't remove above check).2) if speed or duplex or speed is set in the qemu command line, then set the
VIRTIO_NET_F_SPEED_DUPLEXand have host control the settings of speed/duplex (with ability of guest to over-write if it wanted to).
I agree - I don't see a reason to touch the speed/duplex values when VIRTIO_NET_F_SPEED_DUPLEX has not been negotiated.
Quoted 78 lines
Quoted 78 lines
Quoted 25 lines
Quoted 11 lines
drivers/net/virtio_net.c +8
-0+u32 speed; +u8 duplex; + + speed =virtio_cread32 (vi->vdev, + offsetof(struct virtio_net_config , + speed)); +if (ethtool_validate_speed (speed)) + vi->speed = speed;drivers/net/virtio_net.c static void virtnet_config_changed_work(struct work_struct *work) · line 2167
Quoted 10 lines
drivers/net/virtio_net.c +7
-0+ duplex =virtio_cread8 (vi->vdev, + offsetof(struct virtio_net_config , + duplex)); +if (ethtool_validate_duplex (duplex)) + vi->duplex = duplex; + } +if (vi->status &VIRTIO_NET_S_LINK_UP ) {netif_carrier_on (vi->dev);netif_tx_wake_all_queues (vi->dev);OK so this handles the case when
VIRTIO_NET_F_STATUSis set, but when it's clear we need to call this fromvirtnet_probe.Quoted 2 lines
I propose moving this chunk to a function and calling from two places.
good point. will update.
Thanks,
-Jason
Quoted 44 lines
Quoted 44 lines
drivers/net/virtio_net.c +2
-1@@ -2796,7 +2812,8 @@ static struct virtio_device_id id_table[] = { VIRTIO_NET_F_CTRL_RX, VIRTIO_NET_F_CTRL_VLAN, \VIRTIO_NET_F_GUEST_ANNOUNCE ,VIRTIO_NET_F_MQ , \VIRTIO_NET_F_CTRL_MAC_ADDR , \ - VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS +VIRTIO_NET_F_MTU ,VIRTIO_NET_F_CTRL_GUEST_OFFLOADS , \ +VIRTIO_NET_F_SPEED_DUPLEX static unsigned int features[] = {VIRTNET_FEATURES ,include/uapi/linux/virtio_net.h +13
-0@@ -57,6 +57,8 @@* Steering */ #define VIRTIO_NET_F_CTRL_MAC_ADDR 23/* Set MAC address */ +#define VIRTIO_NET_F_SPEED_DUPLEX 63/* Device set linkspeed and duplex */ +#ifndef VIRTIO_NET_NO_LEGACY #define VIRTIO_NET_F_GSO 6/* Host handles pkts w/ any GSO type */ #endif /* VIRTIO_NET_NO_LEGACY */ @@ -76,6 +78,17 @@ struct virtio_net_config {__u16 max_virtqueue_pairs;/* Default maximum transmit unit advice */ __u16 mtu; +/* +* speed, in units of 1Mb. All values 0 to INT_MAX are legal. +* Any other value stands for unknown. +*/ +__u32 speed; +/* +* 0x00 - half duplex +* 0x01 - full duplex +* Any other value stands for unknown. +*/ +__u8 duplex; } __attribute__((packed));/* Signature · 1 line
2.6.1
Re: [PATCH net-next v3 1/3] virtio_net: propagate linkspeed/duplex settings from the hypervisor
In reply to Michael S. TsirkinQuoted 54 lines · Michael S. Tsirkin
Quoted 42 lines · Jason Baron
Quoted 32 lines · Michael S. Tsirkin
Quoted 31 lines · Jason Baron
The ability to set speed and duplex for
virtio_netis useful in various scenarios as described here:16032be
virtio_net: add ethtool support for set and get of settingsHowever, it would be nice to be able to set this from the hypervisor, such that
virtio_netdoesn't require custom guest ethtool commands.Introduce a new feature flag,
VIRTIO_NET_F_SPEED_DUPLEX, which allows the hypervisor to export a linkspeed and duplex setting. The user can subsequently overwrite it later if desired via: 'ethtool -s'.Note that
VIRTIO_NET_F_SPEED_DUPLEXis defined as bit 63, the intention is that device feature bits are to grow down from bit 63, since the transports are starting from bit 24 and growing up.Signed-off-by Jason Baron <[email protected]>Cc "Michael S. Tsirkin" <[email protected]>Cc Jason Wang <[email protected]>--- drivers/net/virtio_net.c | 19 ++++++++++++++++++- include/uapi/linux/virtio_net.h | 13 +++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-)drivers/net/virtio_net.c +0
-0@@ -2146,6 +2146,22 @@ static void virtnet_config_changed_work(struct work_struct *work)Quoted 6 lines
drivers/net/virtio_net.c static void virtnet_config_changed_work(struct work_struct *work) · line 2149
Quoted 3 lines
drivers/net/virtio_net.c +1
-0vi->status = v; +if (virtio_has_feature (vi->vdev,VIRTIO_NET_F_SPEED_DUPLEX )) {BTW we can avoid this read for when link goes down. Not a big deal but still.
Quoted 7 lines
So you are saying that we can just set
vi->speedandvi->duplexto 'unknown' when the link goes down and not check for the presence ofVIRTIO_NET_F_SPEED_DUPLEX?If so, that could over-write what the user may have configured in the guest via 'ethtool -s' when the link goes down, so that would be a change in behavior, but perhaps that is ok?
No - what I am saying is that your patch overwrites the values set by user when link goes down.
Quoted 6 lines
I suggest limiting this call to when
if (
vi->status&VIRTIO_NET_S_LINK_UP)and then the values are overwritten when link goes up which seems closer to what a user might expect.
ok, will update this for v4.
Thanks,
-Jason
Quoted 91 lines
Quoted 91 lines
I think I would prefer to have the link down event still check for
VIRTIO_NET_F_SPEED_DUPLEXbefore changing speed/duplex. That way we still have 2 modes for updating the fields:1) completely guest controlled. Same as we have now and host does not change any values and does not set
VIRTIO_NET_F_SPEED_DUPLEXflag (hence don't remove above check).2) if speed or duplex or speed is set in the qemu command line, then set the
VIRTIO_NET_F_SPEED_DUPLEXand have host control the settings of speed/duplex (with ability of guest to over-write if it wanted to).Quoted 23 lines
Quoted 9 lines
drivers/net/virtio_net.c +8
-0+u32 speed; +u8 duplex; + + speed =virtio_cread32 (vi->vdev, + offsetof(struct virtio_net_config , + speed)); +if (ethtool_validate_speed (speed)) + vi->speed = speed;drivers/net/virtio_net.c static void virtnet_config_changed_work(struct work_struct *work) · line 2167
Quoted 10 lines
drivers/net/virtio_net.c +7
-0+ duplex =virtio_cread8 (vi->vdev, + offsetof(struct virtio_net_config , + duplex)); +if (ethtool_validate_duplex (duplex)) + vi->duplex = duplex; + } +if (vi->status &VIRTIO_NET_S_LINK_UP ) {netif_carrier_on (vi->dev);netif_tx_wake_all_queues (vi->dev);OK so this handles the case when
VIRTIO_NET_F_STATUSis set, but when it's clear we need to call this fromvirtnet_probe.Quoted 2 lines
I propose moving this chunk to a function and calling from two places.
good point. will update.
Thanks,
-Jason
Quoted 44 lines
Quoted 44 lines
drivers/net/virtio_net.c +2
-1@@ -2796,7 +2812,8 @@ static struct virtio_device_id id_table[] = { VIRTIO_NET_F_CTRL_RX, VIRTIO_NET_F_CTRL_VLAN, \VIRTIO_NET_F_GUEST_ANNOUNCE ,VIRTIO_NET_F_MQ , \VIRTIO_NET_F_CTRL_MAC_ADDR , \ - VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS +VIRTIO_NET_F_MTU ,VIRTIO_NET_F_CTRL_GUEST_OFFLOADS , \ +VIRTIO_NET_F_SPEED_DUPLEX static unsigned int features[] = {VIRTNET_FEATURES ,include/uapi/linux/virtio_net.h +13
-0@@ -57,6 +57,8 @@* Steering */ #define VIRTIO_NET_F_CTRL_MAC_ADDR 23/* Set MAC address */ +#define VIRTIO_NET_F_SPEED_DUPLEX 63/* Device set linkspeed and duplex */ +#ifndef VIRTIO_NET_NO_LEGACY #define VIRTIO_NET_F_GSO 6/* Host handles pkts w/ any GSO type */ #endif /* VIRTIO_NET_NO_LEGACY */ @@ -76,6 +78,17 @@ struct virtio_net_config {__u16 max_virtqueue_pairs;/* Default maximum transmit unit advice */ __u16 mtu; +/* +* speed, in units of 1Mb. All values 0 to INT_MAX are legal. +* Any other value stands for unknown. +*/ +__u32 speed; +/* +* 0x00 - half duplex +* 0x01 - full duplex +* Any other value stands for unknown. +*/ +__u8 duplex; } __attribute__((packed));/* Signature · 1 line
2.6.1
Re: [PATCH net-next v3 1/3] virtio_net: propagate linkspeed/duplex settings from the hypervisor
In reply to Michael S. TsirkinQuoted 54 lines · Michael S. Tsirkin
Quoted 42 lines · Jason Baron
Quoted 32 lines · Michael S. Tsirkin
Quoted 31 lines · Jason Baron
The ability to set speed and duplex for
virtio_netis useful in various scenarios as described here:16032be
virtio_net: add ethtool support for set and get of settingsHowever, it would be nice to be able to set this from the hypervisor, such that
virtio_netdoesn't require custom guest ethtool commands.Introduce a new feature flag,
VIRTIO_NET_F_SPEED_DUPLEX, which allows the hypervisor to export a linkspeed and duplex setting. The user can subsequently overwrite it later if desired via: 'ethtool -s'.Note that
VIRTIO_NET_F_SPEED_DUPLEXis defined as bit 63, the intention is that device feature bits are to grow down from bit 63, since the transports are starting from bit 24 and growing up.Signed-off-by Jason Baron <[email protected]>Cc "Michael S. Tsirkin" <[email protected]>Cc Jason Wang <[email protected]>--- drivers/net/virtio_net.c | 19 ++++++++++++++++++- include/uapi/linux/virtio_net.h | 13 +++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-)drivers/net/virtio_net.c +0
-0@@ -2146,6 +2146,22 @@ static void virtnet_config_changed_work(struct work_struct *work)Quoted 6 lines
drivers/net/virtio_net.c static void virtnet_config_changed_work(struct work_struct *work) · line 2149
Quoted 3 lines
drivers/net/virtio_net.c +1
-0vi->status = v; +if (virtio_has_feature (vi->vdev,VIRTIO_NET_F_SPEED_DUPLEX )) {BTW we can avoid this read for when link goes down. Not a big deal but still.
Quoted 7 lines
So you are saying that we can just set
vi->speedandvi->duplexto 'unknown' when the link goes down and not check for the presence ofVIRTIO_NET_F_SPEED_DUPLEX?If so, that could over-write what the user may have configured in the guest via 'ethtool -s' when the link goes down, so that would be a change in behavior, but perhaps that is ok?
No - what I am saying is that your patch overwrites the values set by user when link goes down.
Quoted 6 lines
I suggest limiting this call to when
if (
vi->status&VIRTIO_NET_S_LINK_UP)and then the values are overwritten when link goes up which seems closer to what a user might expect.
ok, will update this for v4.
Thanks,
-Jason
Quoted 91 lines
Quoted 91 lines
I think I would prefer to have the link down event still check for
VIRTIO_NET_F_SPEED_DUPLEXbefore changing speed/duplex. That way we still have 2 modes for updating the fields:1) completely guest controlled. Same as we have now and host does not change any values and does not set
VIRTIO_NET_F_SPEED_DUPLEXflag (hence don't remove above check).2) if speed or duplex or speed is set in the qemu command line, then set the
VIRTIO_NET_F_SPEED_DUPLEXand have host control the settings of speed/duplex (with ability of guest to over-write if it wanted to).Quoted 23 lines
Quoted 9 lines
drivers/net/virtio_net.c +8
-0+u32 speed; +u8 duplex; + + speed =virtio_cread32 (vi->vdev, + offsetof(struct virtio_net_config , + speed)); +if (ethtool_validate_speed (speed)) + vi->speed = speed;drivers/net/virtio_net.c static void virtnet_config_changed_work(struct work_struct *work) · line 2167
Quoted 10 lines
drivers/net/virtio_net.c +7
-0+ duplex =virtio_cread8 (vi->vdev, + offsetof(struct virtio_net_config , + duplex)); +if (ethtool_validate_duplex (duplex)) + vi->duplex = duplex; + } +if (vi->status &VIRTIO_NET_S_LINK_UP ) {netif_carrier_on (vi->dev);netif_tx_wake_all_queues (vi->dev);OK so this handles the case when
VIRTIO_NET_F_STATUSis set, but when it's clear we need to call this fromvirtnet_probe.Quoted 2 lines
I propose moving this chunk to a function and calling from two places.
good point. will update.
Thanks,
-Jason
Quoted 44 lines
Quoted 44 lines
drivers/net/virtio_net.c +2
-1@@ -2796,7 +2812,8 @@ static struct virtio_device_id id_table[] = { VIRTIO_NET_F_CTRL_RX, VIRTIO_NET_F_CTRL_VLAN, \VIRTIO_NET_F_GUEST_ANNOUNCE ,VIRTIO_NET_F_MQ , \VIRTIO_NET_F_CTRL_MAC_ADDR , \ - VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS +VIRTIO_NET_F_MTU ,VIRTIO_NET_F_CTRL_GUEST_OFFLOADS , \ +VIRTIO_NET_F_SPEED_DUPLEX static unsigned int features[] = {VIRTNET_FEATURES ,include/uapi/linux/virtio_net.h +13
-0@@ -57,6 +57,8 @@* Steering */ #define VIRTIO_NET_F_CTRL_MAC_ADDR 23/* Set MAC address */ +#define VIRTIO_NET_F_SPEED_DUPLEX 63/* Device set linkspeed and duplex */ +#ifndef VIRTIO_NET_NO_LEGACY #define VIRTIO_NET_F_GSO 6/* Host handles pkts w/ any GSO type */ #endif /* VIRTIO_NET_NO_LEGACY */ @@ -76,6 +78,17 @@ struct virtio_net_config {__u16 max_virtqueue_pairs;/* Default maximum transmit unit advice */ __u16 mtu; +/* +* speed, in units of 1Mb. All values 0 to INT_MAX are legal. +* Any other value stands for unknown. +*/ +__u32 speed; +/* +* 0x00 - half duplex +* 0x01 - full duplex +* Any other value stands for unknown. +*/ +__u8 duplex; } __attribute__((packed));/* Signature · 1 line
2.6.1
PATCH v3 1/3 virtio_net: propagate linkspeed/duplex settings from the hypervisor
In reply to Jason BaronThe ability to set speed and duplex for virtio_net is useful in various scenarios as described here:
16032be virtio_net: add ethtool support for set and get of settings
However, it would be nice to be able to set this from the hypervisor, such that virtio_net doesn't require custom guest ethtool commands.
Introduce a new feature flag, VIRTIO_NET_F_SPEED_DUPLEX, which allows the hypervisor to export a linkspeed and duplex setting. The user can subsequently overwrite it later if desired via: 'ethtool -s'.
Note that VIRTIO_NET_F_SPEED_DUPLEX is defined as bit 63, the intention is that device feature bits are to grow down from bit 63, since the transports are starting from bit 24 and growing up.
--- drivers/net/virtio_net.c | 19 ++++++++++++++++++- include/uapi/linux/virtio_net.h | 13 +++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-)
drivers/net/virtio_net.c +18 -1
@@ -2146,6 +2146,22 @@ static void virtnet_config_changed_work(struct work_struct *work) vi->status = v; +if (virtio_has_feature (vi->vdev,VIRTIO_NET_F_SPEED_DUPLEX )) { +u32 speed; +u8 duplex; + + speed =virtio_cread32 (vi->vdev, + offsetof(struct virtio_net_config , + speed)); +if (ethtool_validate_speed (speed)) + vi->speed = speed; + duplex =virtio_cread8 (vi->vdev, + offsetof(struct virtio_net_config , + duplex)); +if (ethtool_validate_duplex (duplex)) + vi->duplex = duplex; + } +if (vi->status &VIRTIO_NET_S_LINK_UP ) {netif_carrier_on (vi->dev);netif_tx_wake_all_queues (vi->dev);
@@ -2796,7 +2812,8 @@ static struct virtio_device_id id_table[] = { VIRTIO_NET_F_CTRL_RX, VIRTIO_NET_F_CTRL_VLAN, \VIRTIO_NET_F_GUEST_ANNOUNCE ,VIRTIO_NET_F_MQ , \VIRTIO_NET_F_CTRL_MAC_ADDR , \ - VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS +VIRTIO_NET_F_MTU ,VIRTIO_NET_F_CTRL_GUEST_OFFLOADS , \ +VIRTIO_NET_F_SPEED_DUPLEX static unsigned int features[] = {VIRTNET_FEATURES ,
include/uapi/linux/virtio_net.h +13 -0
@@ -57,6 +57,8 @@* Steering */ #define VIRTIO_NET_F_CTRL_MAC_ADDR 23/* Set MAC address */ +#define VIRTIO_NET_F_SPEED_DUPLEX 63/* Device set linkspeed and duplex */ +#ifndef VIRTIO_NET_NO_LEGACY #define VIRTIO_NET_F_GSO 6/* Host handles pkts w/ any GSO type */ #endif /* VIRTIO_NET_NO_LEGACY */
@@ -76,6 +78,17 @@ struct virtio_net_config {__u16 max_virtqueue_pairs;/* Default maximum transmit unit advice */ __u16 mtu; +/* +* speed, in units of 1Mb. All values 0 to INT_MAX are legal. +* Any other value stands for unknown. +*/ +__u32 speed; +/* +* 0x00 - half duplex +* 0x01 - full duplex +* Any other value stands for unknown. +*/ +__u8 duplex; } __attribute__((packed));/*
Signature · 1 line
2.6.1
PATCH v3 2/3 qemu: virtio-net: use 64-bit values for feature flags
In reply to Jason BaronIn prepartion for using some of the high order feature bits, make sure that virtio-net uses 64-bit values everywhere.
--- hw/net/virtio-net.c | 54 +++++++++++++++++++++--------------------- include/hw/virtio/virtio-net.h | 2 +- 2 files changed, 28 insertions(+), 28 deletions(-)
hw/net/virtio-net.c +27 -27
@@ -48,18 +48,18 @@ (offsetof(container, field) + sizeof(((container *)0)->field))typedef struct VirtIOFeature { -uint32_t flags; +uint64_t flags;size_t end; }VirtIOFeature ;static VirtIOFeature feature_sizes[] = { - {.flags =1 <<VIRTIO_NET_F_MAC , + {.flags =1ULL <<VIRTIO_NET_F_MAC , .end = endof(struct virtio_net_config, mac)}, - {.flags =1 <<VIRTIO_NET_F_STATUS , + {.flags =1ULL <<VIRTIO_NET_F_STATUS , .end = endof(struct virtio_net_config, status)}, - {.flags =1 <<VIRTIO_NET_F_MQ , + {.flags =1ULL <<VIRTIO_NET_F_MQ , .end = endof(struct virtio_net_config, max_virtqueue_pairs)}, - {.flags =1 <<VIRTIO_NET_F_MTU , + {.flags =1ULL <<VIRTIO_NET_F_MTU , .end = endof(struct virtio_net_config, mtu)}, {} };
@@ -1938,7 +1938,7 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp)int i;if (n->net_conf.mtu) { - n->host_features |= (0x1 <<VIRTIO_NET_F_MTU ); + n->host_features |= (1ULL <<VIRTIO_NET_F_MTU ); }virtio_net_set_config_size (n, n->host_features);
@@ -2109,45 +2109,45 @@ static const VMStateDescription vmstate_virtio_net = { };static Property virtio_net_properties[] = { -DEFINE_PROP_BIT ("csum" , VirtIONet, host_features,VIRTIO_NET_F_CSUM , true), -DEFINE_PROP_BIT ("guest_csum" , VirtIONet, host_features, +DEFINE_PROP_BIT64 ("csum" , VirtIONet, host_features,VIRTIO_NET_F_CSUM , true), +DEFINE_PROP_BIT64 ("guest_csum" , VirtIONet, host_features,VIRTIO_NET_F_GUEST_CSUM , true), -DEFINE_PROP_BIT ("gso" , VirtIONet, host_features,VIRTIO_NET_F_GSO , true), -DEFINE_PROP_BIT ("guest_tso4" , VirtIONet, host_features, +DEFINE_PROP_BIT64 ("gso" , VirtIONet, host_features,VIRTIO_NET_F_GSO , true), +DEFINE_PROP_BIT64 ("guest_tso4" , VirtIONet, host_features,VIRTIO_NET_F_GUEST_TSO4 , true), -DEFINE_PROP_BIT ("guest_tso6" , VirtIONet, host_features, +DEFINE_PROP_BIT64 ("guest_tso6" , VirtIONet, host_features,VIRTIO_NET_F_GUEST_TSO6 , true), -DEFINE_PROP_BIT ("guest_ecn" , VirtIONet, host_features, +DEFINE_PROP_BIT64 ("guest_ecn" , VirtIONet, host_features,VIRTIO_NET_F_GUEST_ECN , true), -DEFINE_PROP_BIT ("guest_ufo" , VirtIONet, host_features, +DEFINE_PROP_BIT64 ("guest_ufo" , VirtIONet, host_features,VIRTIO_NET_F_GUEST_UFO , true), -DEFINE_PROP_BIT ("guest_announce" , VirtIONet, host_features, +DEFINE_PROP_BIT64 ("guest_announce" , VirtIONet, host_features,VIRTIO_NET_F_GUEST_ANNOUNCE , true), -DEFINE_PROP_BIT ("host_tso4" , VirtIONet, host_features, +DEFINE_PROP_BIT64 ("host_tso4" , VirtIONet, host_features,VIRTIO_NET_F_HOST_TSO4 , true), -DEFINE_PROP_BIT ("host_tso6" , VirtIONet, host_features, +DEFINE_PROP_BIT64 ("host_tso6" , VirtIONet, host_features,VIRTIO_NET_F_HOST_TSO6 , true), -DEFINE_PROP_BIT ("host_ecn" , VirtIONet, host_features, +DEFINE_PROP_BIT64 ("host_ecn" , VirtIONet, host_features,VIRTIO_NET_F_HOST_ECN , true), -DEFINE_PROP_BIT ("host_ufo" , VirtIONet, host_features, +DEFINE_PROP_BIT64 ("host_ufo" , VirtIONet, host_features,VIRTIO_NET_F_HOST_UFO , true), -DEFINE_PROP_BIT ("mrg_rxbuf" , VirtIONet, host_features, +DEFINE_PROP_BIT64 ("mrg_rxbuf" , VirtIONet, host_features,VIRTIO_NET_F_MRG_RXBUF , true), -DEFINE_PROP_BIT ("status" , VirtIONet, host_features, +DEFINE_PROP_BIT64 ("status" , VirtIONet, host_features,VIRTIO_NET_F_STATUS , true), -DEFINE_PROP_BIT ("ctrl_vq" , VirtIONet, host_features, +DEFINE_PROP_BIT64 ("ctrl_vq" , VirtIONet, host_features,VIRTIO_NET_F_CTRL_VQ , true), -DEFINE_PROP_BIT ("ctrl_rx" , VirtIONet, host_features, +DEFINE_PROP_BIT64 ("ctrl_rx" , VirtIONet, host_features,VIRTIO_NET_F_CTRL_RX , true), -DEFINE_PROP_BIT ("ctrl_vlan" , VirtIONet, host_features, +DEFINE_PROP_BIT64 ("ctrl_vlan" , VirtIONet, host_features,VIRTIO_NET_F_CTRL_VLAN , true), -DEFINE_PROP_BIT ("ctrl_rx_extra" , VirtIONet, host_features, +DEFINE_PROP_BIT64 ("ctrl_rx_extra" , VirtIONet, host_features,VIRTIO_NET_F_CTRL_RX_EXTRA , true), -DEFINE_PROP_BIT ("ctrl_mac_addr" , VirtIONet, host_features, +DEFINE_PROP_BIT64 ("ctrl_mac_addr" , VirtIONet, host_features,VIRTIO_NET_F_CTRL_MAC_ADDR , true), -DEFINE_PROP_BIT ("ctrl_guest_offloads" , VirtIONet, host_features, +DEFINE_PROP_BIT64 ("ctrl_guest_offloads" , VirtIONet, host_features,VIRTIO_NET_F_CTRL_GUEST_OFFLOADS , true), -DEFINE_PROP_BIT ("mq" , VirtIONet, host_features,VIRTIO_NET_F_MQ , false), +DEFINE_PROP_BIT64 ("mq" , VirtIONet, host_features,VIRTIO_NET_F_MQ , false),DEFINE_NIC_PROPERTIES (VirtIONet, nic_conf),DEFINE_PROP_UINT32 ("x-txtimer" , VirtIONet, net_conf.txtimer,TX_TIMER_INTERVAL ),
include/hw/virtio/virtio-net.h +1 -1
@@ -67,7 +67,7 @@ typedef struct VirtIONet {uint32_t has_vnet_hdr;size_t host_hdr_len;size_t guest_hdr_len; -uint32_t host_features; +uint64_t host_features;uint8_t has_ufo;uint32_t mergeable_rx_bufs;uint8_t promisc;
Signature · 1 line
2.6.1
PATCH v3 2/3 qemu: virtio-net: use 64-bit values for feature flags
In reply to Jason BaronIn prepartion for using some of the high order feature bits, make sure that virtio-net uses 64-bit values everywhere.
--- hw/net/virtio-net.c | 54 +++++++++++++++++++++--------------------- include/hw/virtio/virtio-net.h | 2 +- 2 files changed, 28 insertions(+), 28 deletions(-)
hw/net/virtio-net.c +27 -27
@@ -48,18 +48,18 @@ (offsetof(container, field) + sizeof(((container *)0)->field))typedef struct VirtIOFeature { -uint32_t flags; +uint64_t flags;size_t end; }VirtIOFeature ;static VirtIOFeature feature_sizes[] = { - {.flags =1 <<VIRTIO_NET_F_MAC , + {.flags =1ULL <<VIRTIO_NET_F_MAC , .end = endof(struct virtio_net_config, mac)}, - {.flags =1 <<VIRTIO_NET_F_STATUS , + {.flags =1ULL <<VIRTIO_NET_F_STATUS , .end = endof(struct virtio_net_config, status)}, - {.flags =1 <<VIRTIO_NET_F_MQ , + {.flags =1ULL <<VIRTIO_NET_F_MQ , .end = endof(struct virtio_net_config, max_virtqueue_pairs)}, - {.flags =1 <<VIRTIO_NET_F_MTU , + {.flags =1ULL <<VIRTIO_NET_F_MTU , .end = endof(struct virtio_net_config, mtu)}, {} };
@@ -1938,7 +1938,7 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp)int i;if (n->net_conf.mtu) { - n->host_features |= (0x1 <<VIRTIO_NET_F_MTU ); + n->host_features |= (1ULL <<VIRTIO_NET_F_MTU ); }virtio_net_set_config_size (n, n->host_features);
@@ -2109,45 +2109,45 @@ static const VMStateDescription vmstate_virtio_net = { };static Property virtio_net_properties[] = { -DEFINE_PROP_BIT ("csum" , VirtIONet, host_features,VIRTIO_NET_F_CSUM , true), -DEFINE_PROP_BIT ("guest_csum" , VirtIONet, host_features, +DEFINE_PROP_BIT64 ("csum" , VirtIONet, host_features,VIRTIO_NET_F_CSUM , true), +DEFINE_PROP_BIT64 ("guest_csum" , VirtIONet, host_features,VIRTIO_NET_F_GUEST_CSUM , true), -DEFINE_PROP_BIT ("gso" , VirtIONet, host_features,VIRTIO_NET_F_GSO , true), -DEFINE_PROP_BIT ("guest_tso4" , VirtIONet, host_features, +DEFINE_PROP_BIT64 ("gso" , VirtIONet, host_features,VIRTIO_NET_F_GSO , true), +DEFINE_PROP_BIT64 ("guest_tso4" , VirtIONet, host_features,VIRTIO_NET_F_GUEST_TSO4 , true), -DEFINE_PROP_BIT ("guest_tso6" , VirtIONet, host_features, +DEFINE_PROP_BIT64 ("guest_tso6" , VirtIONet, host_features,VIRTIO_NET_F_GUEST_TSO6 , true), -DEFINE_PROP_BIT ("guest_ecn" , VirtIONet, host_features, +DEFINE_PROP_BIT64 ("guest_ecn" , VirtIONet, host_features,VIRTIO_NET_F_GUEST_ECN , true), -DEFINE_PROP_BIT ("guest_ufo" , VirtIONet, host_features, +DEFINE_PROP_BIT64 ("guest_ufo" , VirtIONet, host_features,VIRTIO_NET_F_GUEST_UFO , true), -DEFINE_PROP_BIT ("guest_announce" , VirtIONet, host_features, +DEFINE_PROP_BIT64 ("guest_announce" , VirtIONet, host_features,VIRTIO_NET_F_GUEST_ANNOUNCE , true), -DEFINE_PROP_BIT ("host_tso4" , VirtIONet, host_features, +DEFINE_PROP_BIT64 ("host_tso4" , VirtIONet, host_features,VIRTIO_NET_F_HOST_TSO4 , true), -DEFINE_PROP_BIT ("host_tso6" , VirtIONet, host_features, +DEFINE_PROP_BIT64 ("host_tso6" , VirtIONet, host_features,VIRTIO_NET_F_HOST_TSO6 , true), -DEFINE_PROP_BIT ("host_ecn" , VirtIONet, host_features, +DEFINE_PROP_BIT64 ("host_ecn" , VirtIONet, host_features,VIRTIO_NET_F_HOST_ECN , true), -DEFINE_PROP_BIT ("host_ufo" , VirtIONet, host_features, +DEFINE_PROP_BIT64 ("host_ufo" , VirtIONet, host_features,VIRTIO_NET_F_HOST_UFO , true), -DEFINE_PROP_BIT ("mrg_rxbuf" , VirtIONet, host_features, +DEFINE_PROP_BIT64 ("mrg_rxbuf" , VirtIONet, host_features,VIRTIO_NET_F_MRG_RXBUF , true), -DEFINE_PROP_BIT ("status" , VirtIONet, host_features, +DEFINE_PROP_BIT64 ("status" , VirtIONet, host_features,VIRTIO_NET_F_STATUS , true), -DEFINE_PROP_BIT ("ctrl_vq" , VirtIONet, host_features, +DEFINE_PROP_BIT64 ("ctrl_vq" , VirtIONet, host_features,VIRTIO_NET_F_CTRL_VQ , true), -DEFINE_PROP_BIT ("ctrl_rx" , VirtIONet, host_features, +DEFINE_PROP_BIT64 ("ctrl_rx" , VirtIONet, host_features,VIRTIO_NET_F_CTRL_RX , true), -DEFINE_PROP_BIT ("ctrl_vlan" , VirtIONet, host_features, +DEFINE_PROP_BIT64 ("ctrl_vlan" , VirtIONet, host_features,VIRTIO_NET_F_CTRL_VLAN , true), -DEFINE_PROP_BIT ("ctrl_rx_extra" , VirtIONet, host_features, +DEFINE_PROP_BIT64 ("ctrl_rx_extra" , VirtIONet, host_features,VIRTIO_NET_F_CTRL_RX_EXTRA , true), -DEFINE_PROP_BIT ("ctrl_mac_addr" , VirtIONet, host_features, +DEFINE_PROP_BIT64 ("ctrl_mac_addr" , VirtIONet, host_features,VIRTIO_NET_F_CTRL_MAC_ADDR , true), -DEFINE_PROP_BIT ("ctrl_guest_offloads" , VirtIONet, host_features, +DEFINE_PROP_BIT64 ("ctrl_guest_offloads" , VirtIONet, host_features,VIRTIO_NET_F_CTRL_GUEST_OFFLOADS , true), -DEFINE_PROP_BIT ("mq" , VirtIONet, host_features,VIRTIO_NET_F_MQ , false), +DEFINE_PROP_BIT64 ("mq" , VirtIONet, host_features,VIRTIO_NET_F_MQ , false),DEFINE_NIC_PROPERTIES (VirtIONet, nic_conf),DEFINE_PROP_UINT32 ("x-txtimer" , VirtIONet, net_conf.txtimer,TX_TIMER_INTERVAL ),
include/hw/virtio/virtio-net.h +1 -1
@@ -67,7 +67,7 @@ typedef struct VirtIONet {uint32_t has_vnet_hdr;size_t host_hdr_len;size_t guest_hdr_len; -uint32_t host_features; +uint64_t host_features;uint8_t has_ufo;uint32_t mergeable_rx_bufs;uint8_t promisc;
Signature · 1 line
2.6.1
PATCH v3 3/3 qemu: add linkspeed and duplex settings to virtio-net
In reply to Jason BaronAlthough linkspeed and duplex can be set in a linux guest via 'ethtool -s', this requires custom ethtool commands for virtio-net by default.
Introduce a new feature flag, VIRTIO_NET_F_SPEED_DUPLEX, which allows the hypervisor to export a linkspeed and duplex setting. The user can subsequently overwrite it later if desired via: 'ethtool -s'.
Linkspeed and duplex settings can be set as:
'-device virtio-net,speed=10000,duplex=full'
where speed is [-1...INT_MAX], and duplex is ["half"|"full"].
--- hw/net/virtio-net.c | 35 +++++++++++++++++++++++++++++ include/hw/virtio/virtio-net.h | 3 +++ include/standard-headers/linux/virtio_net.h | 13 +++++++++++ 3 files changed, 51 insertions(+)
hw/net/virtio-net.c +35 -0
@@ -40,6 +40,12 @@#define VIRTIO_NET_RX_QUEUE_MIN_SIZE VIRTIO_NET_RX_QUEUE_DEFAULT_SIZE#define VIRTIO_NET_TX_QUEUE_MIN_SIZE VIRTIO_NET_TX_QUEUE_DEFAULT_SIZE +/* duplex and speed */ +#define DUPLEX_UNKNOWN 0xff +#define DUPLEX_HALF 0x00 +#define DUPLEX_FULL 0x01 +#define SPEED_UNKNOWN -1 +/* * Calculate the number of bytes up to and including the given 'field' of * 'container'.
@@ -61,6 +67,8 @@ static VirtIOFeature feature_sizes[] = { .end = endof(struct virtio_net_config, max_virtqueue_pairs)}, {.flags = 1ULL << VIRTIO_NET_F_MTU, .end = endof(struct virtio_net_config, mtu)}, + {.flags = 1ULL << VIRTIO_NET_F_SPEED_DUPLEX, + .end = endof(struct virtio_net_config, duplex)}, {} };
@@ -89,6 +97,14 @@ static void virtio_net_get_config(VirtIODevice *vdev, uint8_t *config)virtio_stw_p (vdev, &netcfg.max_virtqueue_pairs, n->max_queues);virtio_stw_p (vdev, &netcfg.mtu, n->net_conf.mtu);memcpy (netcfg.mac, n->mac,ETH_ALEN ); +if (n->status &VIRTIO_NET_S_LINK_UP ) { +virtio_stl_p (vdev, &netcfg.speed, n->net_conf.speed); + netcfg.duplex = n->net_conf.duplex; + }else { +virtio_stl_p (vdev, &netcfg.speed,SPEED_UNKNOWN ); + netcfg.duplex =DUPLEX_UNKNOWN ; + } +memcpy (config, &netcfg, n->config_size); }
@@ -1941,6 +1957,23 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp) n->host_features |= (1ULL <<VIRTIO_NET_F_MTU ); } + n->host_features |= (1ULL <<VIRTIO_NET_F_SPEED_DUPLEX ); +if (n->net_conf.duplex_str) { +if (strncmp (n->net_conf.duplex_str,"half" ,5 ) ==0 ) { + n->net_conf.duplex =DUPLEX_HALF ; + }else if (strncmp (n->net_conf.duplex_str,"full" ,5 ) ==0 ) { + n->net_conf.duplex =DUPLEX_FULL ; + }else { +error_setg (errp,"'duplex' must be 'half' or 'full'" ); + } + }else { + n->net_conf.duplex =DUPLEX_UNKNOWN ; + } +if (n->net_conf.speed <SPEED_UNKNOWN ) { +error_setg (errp,"'speed' must be between -1 (SPEED_UNKOWN) and " +"INT_MAX" ); + } +virtio_net_set_config_size (n, n->host_features);virtio_init (vdev,"virtio-net" ,VIRTIO_ID_NET , n->config_size);
@@ -2160,6 +2193,8 @@ static Property virtio_net_properties[] = {DEFINE_PROP_UINT16 ("host_mtu" , VirtIONet, net_conf.mtu,0 ),DEFINE_PROP_BOOL ("x-mtu-bypass-backend" , VirtIONet, mtu_bypass_backend, true), +DEFINE_PROP_INT32 ("speed" , VirtIONet, net_conf.speed,SPEED_UNKNOWN ), +DEFINE_PROP_STRING ("duplex" , VirtIONet, net_conf.duplex_str), DEFINE_PROP_END_OF_LIST(), };
include/hw/virtio/virtio-net.h +3 -0
@@ -38,6 +38,9 @@ typedef struct virtio_net_confuint16_t rx_queue_size;uint16_t tx_queue_size;uint16_t mtu; +int32_t speed; +char *duplex_str; +uint8_t duplex; } virtio_net_conf;/* Maximum packet size we can receive from tap device: header + 64k */
include/standard-headers/linux/virtio_net.h +13 -0
@@ -57,6 +57,8 @@* Steering */ #define VIRTIO_NET_F_CTRL_MAC_ADDR 23/* Set MAC address */ +#define VIRTIO_NET_F_SPEED_DUPLEX 63/* Device set linkspeed and duplex */ +#ifndef VIRTIO_NET_NO_LEGACY #define VIRTIO_NET_F_GSO 6/* Host handles pkts w/ any GSO type */ #endif /* VIRTIO_NET_NO_LEGACY */
@@ -76,6 +78,17 @@ struct virtio_net_config {uint16_t max_virtqueue_pairs;/* Default maximum transmit unit advice */ uint16_t mtu; +/* +* speed, in units of 1Mb. All values 0 to INT_MAX are legal. +* Any other value stands for unknown. +*/ +uint32_t speed; +/* +* 0x00 - half duplex +* 0x01 - full duplex +* Any other value stands for unknown. +*/ +uint8_t duplex; }QEMU_PACKED ;/*
PATCH v3 3/3 qemu: add linkspeed and duplex settings to virtio-net
In reply to Jason BaronAlthough linkspeed and duplex can be set in a linux guest via 'ethtool -s', this requires custom ethtool commands for virtio-net by default.
Introduce a new feature flag, VIRTIO_NET_F_SPEED_DUPLEX, which allows the hypervisor to export a linkspeed and duplex setting. The user can subsequently overwrite it later if desired via: 'ethtool -s'.
Linkspeed and duplex settings can be set as:
'-device virtio-net,speed=10000,duplex=full'
where speed is [-1...INT_MAX], and duplex is ["half"|"full"].
--- hw/net/virtio-net.c | 35 +++++++++++++++++++++++++++++ include/hw/virtio/virtio-net.h | 3 +++ include/standard-headers/linux/virtio_net.h | 13 +++++++++++ 3 files changed, 51 insertions(+)
hw/net/virtio-net.c +35 -0
@@ -40,6 +40,12 @@#define VIRTIO_NET_RX_QUEUE_MIN_SIZE VIRTIO_NET_RX_QUEUE_DEFAULT_SIZE#define VIRTIO_NET_TX_QUEUE_MIN_SIZE VIRTIO_NET_TX_QUEUE_DEFAULT_SIZE +/* duplex and speed */ +#define DUPLEX_UNKNOWN 0xff +#define DUPLEX_HALF 0x00 +#define DUPLEX_FULL 0x01 +#define SPEED_UNKNOWN -1 +/* * Calculate the number of bytes up to and including the given 'field' of * 'container'.
@@ -61,6 +67,8 @@ static VirtIOFeature feature_sizes[] = { .end = endof(struct virtio_net_config, max_virtqueue_pairs)}, {.flags = 1ULL << VIRTIO_NET_F_MTU, .end = endof(struct virtio_net_config, mtu)}, + {.flags = 1ULL << VIRTIO_NET_F_SPEED_DUPLEX, + .end = endof(struct virtio_net_config, duplex)}, {} };
@@ -89,6 +97,14 @@ static void virtio_net_get_config(VirtIODevice *vdev, uint8_t *config)virtio_stw_p (vdev, &netcfg.max_virtqueue_pairs, n->max_queues);virtio_stw_p (vdev, &netcfg.mtu, n->net_conf.mtu);memcpy (netcfg.mac, n->mac,ETH_ALEN ); +if (n->status &VIRTIO_NET_S_LINK_UP ) { +virtio_stl_p (vdev, &netcfg.speed, n->net_conf.speed); + netcfg.duplex = n->net_conf.duplex; + }else { +virtio_stl_p (vdev, &netcfg.speed,SPEED_UNKNOWN ); + netcfg.duplex =DUPLEX_UNKNOWN ; + } +memcpy (config, &netcfg, n->config_size); }
@@ -1941,6 +1957,23 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp) n->host_features |= (1ULL <<VIRTIO_NET_F_MTU ); } + n->host_features |= (1ULL <<VIRTIO_NET_F_SPEED_DUPLEX ); +if (n->net_conf.duplex_str) { +if (strncmp (n->net_conf.duplex_str,"half" ,5 ) ==0 ) { + n->net_conf.duplex =DUPLEX_HALF ; + }else if (strncmp (n->net_conf.duplex_str,"full" ,5 ) ==0 ) { + n->net_conf.duplex =DUPLEX_FULL ; + }else { +error_setg (errp,"'duplex' must be 'half' or 'full'" ); + } + }else { + n->net_conf.duplex =DUPLEX_UNKNOWN ; + } +if (n->net_conf.speed <SPEED_UNKNOWN ) { +error_setg (errp,"'speed' must be between -1 (SPEED_UNKOWN) and " +"INT_MAX" ); + } +virtio_net_set_config_size (n, n->host_features);virtio_init (vdev,"virtio-net" ,VIRTIO_ID_NET , n->config_size);
@@ -2160,6 +2193,8 @@ static Property virtio_net_properties[] = {DEFINE_PROP_UINT16 ("host_mtu" , VirtIONet, net_conf.mtu,0 ),DEFINE_PROP_BOOL ("x-mtu-bypass-backend" , VirtIONet, mtu_bypass_backend, true), +DEFINE_PROP_INT32 ("speed" , VirtIONet, net_conf.speed,SPEED_UNKNOWN ), +DEFINE_PROP_STRING ("duplex" , VirtIONet, net_conf.duplex_str), DEFINE_PROP_END_OF_LIST(), };
include/hw/virtio/virtio-net.h +3 -0
@@ -38,6 +38,9 @@ typedef struct virtio_net_confuint16_t rx_queue_size;uint16_t tx_queue_size;uint16_t mtu; +int32_t speed; +char *duplex_str; +uint8_t duplex; } virtio_net_conf;/* Maximum packet size we can receive from tap device: header + 64k */
include/standard-headers/linux/virtio_net.h +13 -0
@@ -57,6 +57,8 @@* Steering */ #define VIRTIO_NET_F_CTRL_MAC_ADDR 23/* Set MAC address */ +#define VIRTIO_NET_F_SPEED_DUPLEX 63/* Device set linkspeed and duplex */ +#ifndef VIRTIO_NET_NO_LEGACY #define VIRTIO_NET_F_GSO 6/* Host handles pkts w/ any GSO type */ #endif /* VIRTIO_NET_NO_LEGACY */
@@ -76,6 +78,17 @@ struct virtio_net_config {uint16_t max_virtqueue_pairs;/* Default maximum transmit unit advice */ uint16_t mtu; +/* +* speed, in units of 1Mb. All values 0 to INT_MAX are legal. +* Any other value stands for unknown. +*/ +uint32_t speed; +/* +* 0x00 - half duplex +* 0x01 - full duplex +* Any other value stands for unknown. +*/ +uint8_t duplex; }QEMU_PACKED ;/*