Re: [Qemu-devel] [PATCH 2/2] qemu: add linkspeed and duplex setting to virtio-net

2017-12-21 Thread Michael S. Tsirkin 
On Thu, Dec 21, 2017 at 02:42:48PM -0500, Jason Baron wrote:
> 
> 
> On 12/20/2017 09:33 AM, Yan Vugenfirer wrote:
> > 
> >> On 20 Dec 2017, at 16:31, Michael S. Tsirkin  wrote:
> >>
> >> On Tue, Dec 19, 2017 at 11:52:39AM -0500, Jason Baron wrote:
> >>>
> >>>
> >>> On 12/19/2017 04:19 AM, Yan Vugenfirer wrote:
> 
> > On 18 Dec 2017, at 18:04, Jason Baron via Qemu-devel
> > mailto:qemu-devel@nongnu.org>> wrote:
> >
> >
> >
> > On 12/18/2017 06:34 AM, Yan Vugenfirer wrote:
> >>
> >>> On 14 Dec 2017, at 21:33, Jason Baron via Qemu-devel
> >>> mailto:qemu-devel@nongnu.org>> wrote:
> >>>
> >>> Although they can be currently set in linux via 'ethtool -s', this
> >>> requires
> >>> guest changes, and thus it would be nice to extend this
> >>> functionality such
> >>> that it can be configured automatically from the host (as other 
> >>> network
> >>> do).
> >>>
> >>> Linkspeed and duplex settings can be set as:
> >>> '-device virtio-net,speed=1,duplex=full'
> >>>
> >>> where speed is [-1...INT_MAX], and duplex is ["half"|"full"].
> >>>
> >>> Signed-off-by: Jason Baron  >>> >
> >>> Cc: "Michael S. Tsirkin" mailto:m...@redhat.com>>
> >>> Cc: Jason Wang mailto:jasow...@redhat.com>>
> >>> ---
> >>> hw/net/virtio-net.c | 29
> >>> +
> >>> include/hw/virtio/virtio-net.h  |  3 +++
> >>> include/standard-headers/linux/virtio_net.h |  4 
> >>> 3 files changed, 36 insertions(+)
> >>>
> >>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> >>> index 38674b0..d63e790 100644
> >>> --- a/hw/net/virtio-net.c
> >>> +++ b/hw/net/virtio-net.c
> >>> @@ -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 defines */
> >>> +#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 = 1 << VIRTIO_NET_F_MTU,
> >>> .end = endof(struct virtio_net_config, mtu)},
> >>> +{.flags = 1 << VIRTIO_NET_F_SPEED_DUPLEX,
> >>> + .end = endof(struct virtio_net_config, duplex)},
> >>>{}
> >>> };
> >>>
> >>> @@ -88,6 +96,8 @@ static void virtio_net_get_config(VirtIODevice
> >>> *vdev, uint8_t *config)
> >>>virtio_stw_p(vdev, &netcfg.status, n->status);
> >>>virtio_stw_p(vdev, &netcfg.max_virtqueue_pairs, n->max_queues);
> >>>virtio_stw_p(vdev, &netcfg.mtu, n->net_conf.mtu);
> >>> +virtio_stl_p(vdev, &netcfg.speed, n->net_conf.speed);
> >>> +netcfg.duplex = n->net_conf.duplex;
> >>>memcpy(netcfg.mac, n->mac, ETH_ALEN);
> >>>memcpy(config, &netcfg, n->config_size);
> >>> }
> >>> @@ -1941,6 +1951,23 @@ static void
> >>> virtio_net_device_realize(DeviceState *dev, Error **errp)
> >>>n->host_features |= (0x1 << VIRTIO_NET_F_MTU);
> >>>}
> >>>
> >>> +n->host_features |= (0x1 << 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 +2187,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),
> >>
> >> From Windows guest perspective I prefer to have some reasonable
> >> defa

Re: [Qemu-devel] [PATCH 2/2] qemu: add linkspeed and duplex setting to virtio-net

2017-12-21 Thread Jason Baron via Qemu-devel 


On 12/20/2017 09:33 AM, Yan Vugenfirer wrote:
> 
>> On 20 Dec 2017, at 16:31, Michael S. Tsirkin  wrote:
>>
>> On Tue, Dec 19, 2017 at 11:52:39AM -0500, Jason Baron wrote:
>>>
>>>
>>> On 12/19/2017 04:19 AM, Yan Vugenfirer wrote:

> On 18 Dec 2017, at 18:04, Jason Baron via Qemu-devel
> mailto:qemu-devel@nongnu.org>> wrote:
>
>
>
> On 12/18/2017 06:34 AM, Yan Vugenfirer wrote:
>>
>>> On 14 Dec 2017, at 21:33, Jason Baron via Qemu-devel
>>> mailto:qemu-devel@nongnu.org>> wrote:
>>>
>>> Although they can be currently set in linux via 'ethtool -s', this
>>> requires
>>> guest changes, and thus it would be nice to extend this
>>> functionality such
>>> that it can be configured automatically from the host (as other network
>>> do).
>>>
>>> Linkspeed and duplex settings can be set as:
>>> '-device virtio-net,speed=1,duplex=full'
>>>
>>> where speed is [-1...INT_MAX], and duplex is ["half"|"full"].
>>>
>>> Signed-off-by: Jason Baron >> >
>>> Cc: "Michael S. Tsirkin" mailto:m...@redhat.com>>
>>> Cc: Jason Wang mailto:jasow...@redhat.com>>
>>> ---
>>> hw/net/virtio-net.c | 29
>>> +
>>> include/hw/virtio/virtio-net.h  |  3 +++
>>> include/standard-headers/linux/virtio_net.h |  4 
>>> 3 files changed, 36 insertions(+)
>>>
>>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
>>> index 38674b0..d63e790 100644
>>> --- a/hw/net/virtio-net.c
>>> +++ b/hw/net/virtio-net.c
>>> @@ -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 defines */
>>> +#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 = 1 << VIRTIO_NET_F_MTU,
>>> .end = endof(struct virtio_net_config, mtu)},
>>> +{.flags = 1 << VIRTIO_NET_F_SPEED_DUPLEX,
>>> + .end = endof(struct virtio_net_config, duplex)},
>>>{}
>>> };
>>>
>>> @@ -88,6 +96,8 @@ static void virtio_net_get_config(VirtIODevice
>>> *vdev, uint8_t *config)
>>>virtio_stw_p(vdev, &netcfg.status, n->status);
>>>virtio_stw_p(vdev, &netcfg.max_virtqueue_pairs, n->max_queues);
>>>virtio_stw_p(vdev, &netcfg.mtu, n->net_conf.mtu);
>>> +virtio_stl_p(vdev, &netcfg.speed, n->net_conf.speed);
>>> +netcfg.duplex = n->net_conf.duplex;
>>>memcpy(netcfg.mac, n->mac, ETH_ALEN);
>>>memcpy(config, &netcfg, n->config_size);
>>> }
>>> @@ -1941,6 +1951,23 @@ static void
>>> virtio_net_device_realize(DeviceState *dev, Error **errp)
>>>n->host_features |= (0x1 << VIRTIO_NET_F_MTU);
>>>}
>>>
>>> +n->host_features |= (0x1 << 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 +2187,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),
>>
>> From Windows guest perspective I prefer to have some reasonable
>> default (10G for example). 
>
>
> hmmm, I didn't want to change/set the default here in case it broke
> something, but I'm ok setting it to some 'reasonable' value - (10G and
> duplex?), if the consensus is that that would be safe.

 OK from my side.
 Thanks.
>>>
>>> I presume your spea

Re: [Qemu-devel] [PATCH 2/2] qemu: add linkspeed and duplex setting to virtio-net

2017-12-20 Thread Yan Vugenfirer 

> On 20 Dec 2017, at 16:31, Michael S. Tsirkin  wrote:
> 
> On Tue, Dec 19, 2017 at 11:52:39AM -0500, Jason Baron wrote:
>> 
>> 
>> On 12/19/2017 04:19 AM, Yan Vugenfirer wrote:
>>> 
 On 18 Dec 2017, at 18:04, Jason Baron via Qemu-devel
 mailto:qemu-devel@nongnu.org>> wrote:
 
 
 
 On 12/18/2017 06:34 AM, Yan Vugenfirer wrote:
> 
>> On 14 Dec 2017, at 21:33, Jason Baron via Qemu-devel
>> mailto:qemu-devel@nongnu.org>> wrote:
>> 
>> Although they can be currently set in linux via 'ethtool -s', this
>> requires
>> guest changes, and thus it would be nice to extend this
>> functionality such
>> that it can be configured automatically from the host (as other network
>> do).
>> 
>> Linkspeed and duplex settings can be set as:
>> '-device virtio-net,speed=1,duplex=full'
>> 
>> where speed is [-1...INT_MAX], and duplex is ["half"|"full"].
>> 
>> Signed-off-by: Jason Baron > >
>> Cc: "Michael S. Tsirkin" mailto:m...@redhat.com>>
>> Cc: Jason Wang mailto:jasow...@redhat.com>>
>> ---
>> hw/net/virtio-net.c | 29
>> +
>> include/hw/virtio/virtio-net.h  |  3 +++
>> include/standard-headers/linux/virtio_net.h |  4 
>> 3 files changed, 36 insertions(+)
>> 
>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
>> index 38674b0..d63e790 100644
>> --- a/hw/net/virtio-net.c
>> +++ b/hw/net/virtio-net.c
>> @@ -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 defines */
>> +#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 = 1 << VIRTIO_NET_F_MTU,
>> .end = endof(struct virtio_net_config, mtu)},
>> +{.flags = 1 << VIRTIO_NET_F_SPEED_DUPLEX,
>> + .end = endof(struct virtio_net_config, duplex)},
>>{}
>> };
>> 
>> @@ -88,6 +96,8 @@ static void virtio_net_get_config(VirtIODevice
>> *vdev, uint8_t *config)
>>virtio_stw_p(vdev, &netcfg.status, n->status);
>>virtio_stw_p(vdev, &netcfg.max_virtqueue_pairs, n->max_queues);
>>virtio_stw_p(vdev, &netcfg.mtu, n->net_conf.mtu);
>> +virtio_stl_p(vdev, &netcfg.speed, n->net_conf.speed);
>> +netcfg.duplex = n->net_conf.duplex;
>>memcpy(netcfg.mac, n->mac, ETH_ALEN);
>>memcpy(config, &netcfg, n->config_size);
>> }
>> @@ -1941,6 +1951,23 @@ static void
>> virtio_net_device_realize(DeviceState *dev, Error **errp)
>>n->host_features |= (0x1 << VIRTIO_NET_F_MTU);
>>}
>> 
>> +n->host_features |= (0x1 << 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 +2187,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),
> 
> From Windows guest perspective I prefer to have some reasonable
> default (10G for example). 
 
 
 hmmm, I didn't want to change/set the default here in case it broke
 something, but I'm ok setting it to some 'reasonable' value - (10G and
 duplex?), if the consensus is that that would be safe.
>>> 
>>> OK from my side.
>>> Thanks.
>> 
>> I presume your speaking for windows - i'm wondering if under linux the
>> virtio device suddenly start showing up as duplex, 10Gbps, will that
>> break anything? I can s

Re: [Qemu-devel] [PATCH 2/2] qemu: add linkspeed and duplex setting to virtio-net

2017-12-20 Thread Yan Vugenfirer 

> On 20 Dec 2017, at 16:31, Michael S. Tsirkin  wrote:
> 
> On Tue, Dec 19, 2017 at 11:52:39AM -0500, Jason Baron wrote:
>> 
>> 
>> On 12/19/2017 04:19 AM, Yan Vugenfirer wrote:
>>> 
 On 18 Dec 2017, at 18:04, Jason Baron via Qemu-devel
 mailto:qemu-devel@nongnu.org>> wrote:
 
 
 
 On 12/18/2017 06:34 AM, Yan Vugenfirer wrote:
> 
>> On 14 Dec 2017, at 21:33, Jason Baron via Qemu-devel
>> mailto:qemu-devel@nongnu.org>> wrote:
>> 
>> Although they can be currently set in linux via 'ethtool -s', this
>> requires
>> guest changes, and thus it would be nice to extend this
>> functionality such
>> that it can be configured automatically from the host (as other network
>> do).
>> 
>> Linkspeed and duplex settings can be set as:
>> '-device virtio-net,speed=1,duplex=full'
>> 
>> where speed is [-1...INT_MAX], and duplex is ["half"|"full"].
>> 
>> Signed-off-by: Jason Baron > >
>> Cc: "Michael S. Tsirkin" mailto:m...@redhat.com>>
>> Cc: Jason Wang mailto:jasow...@redhat.com>>
>> ---
>> hw/net/virtio-net.c | 29
>> +
>> include/hw/virtio/virtio-net.h  |  3 +++
>> include/standard-headers/linux/virtio_net.h |  4 
>> 3 files changed, 36 insertions(+)
>> 
>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
>> index 38674b0..d63e790 100644
>> --- a/hw/net/virtio-net.c
>> +++ b/hw/net/virtio-net.c
>> @@ -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 defines */
>> +#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 = 1 << VIRTIO_NET_F_MTU,
>> .end = endof(struct virtio_net_config, mtu)},
>> +{.flags = 1 << VIRTIO_NET_F_SPEED_DUPLEX,
>> + .end = endof(struct virtio_net_config, duplex)},
>>{}
>> };
>> 
>> @@ -88,6 +96,8 @@ static void virtio_net_get_config(VirtIODevice
>> *vdev, uint8_t *config)
>>virtio_stw_p(vdev, &netcfg.status, n->status);
>>virtio_stw_p(vdev, &netcfg.max_virtqueue_pairs, n->max_queues);
>>virtio_stw_p(vdev, &netcfg.mtu, n->net_conf.mtu);
>> +virtio_stl_p(vdev, &netcfg.speed, n->net_conf.speed);
>> +netcfg.duplex = n->net_conf.duplex;
>>memcpy(netcfg.mac, n->mac, ETH_ALEN);
>>memcpy(config, &netcfg, n->config_size);
>> }
>> @@ -1941,6 +1951,23 @@ static void
>> virtio_net_device_realize(DeviceState *dev, Error **errp)
>>n->host_features |= (0x1 << VIRTIO_NET_F_MTU);
>>}
>> 
>> +n->host_features |= (0x1 << 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 +2187,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),
> 
> From Windows guest perspective I prefer to have some reasonable
> default (10G for example). 
 
 
 hmmm, I didn't want to change/set the default here in case it broke
 something, but I'm ok setting it to some 'reasonable' value - (10G and
 duplex?), if the consensus is that that would be safe.
>>> 
>>> OK from my side.
>>> Thanks.
>> 
>> I presume your speaking for windows - i'm wondering if under linux the
>> virtio device suddenly start showing up as duplex, 10Gbps, will that
>> break anything? I can s

Re: [Qemu-devel] [PATCH 2/2] qemu: add linkspeed and duplex setting to virtio-net

2017-12-20 Thread Michael S. Tsirkin 
On Tue, Dec 19, 2017 at 11:52:39AM -0500, Jason Baron wrote:
> 
> 
> On 12/19/2017 04:19 AM, Yan Vugenfirer wrote:
> > 
> >> On 18 Dec 2017, at 18:04, Jason Baron via Qemu-devel
> >> mailto:qemu-devel@nongnu.org>> wrote:
> >>
> >>
> >>
> >> On 12/18/2017 06:34 AM, Yan Vugenfirer wrote:
> >>>
>  On 14 Dec 2017, at 21:33, Jason Baron via Qemu-devel
>  mailto:qemu-devel@nongnu.org>> wrote:
> 
>  Although they can be currently set in linux via 'ethtool -s', this
>  requires
>  guest changes, and thus it would be nice to extend this
>  functionality such
>  that it can be configured automatically from the host (as other network
>  do).
> 
>  Linkspeed and duplex settings can be set as:
>  '-device virtio-net,speed=1,duplex=full'
> 
>  where speed is [-1...INT_MAX], and duplex is ["half"|"full"].
> 
>  Signed-off-by: Jason Baron   >
>  Cc: "Michael S. Tsirkin" mailto:m...@redhat.com>>
>  Cc: Jason Wang mailto:jasow...@redhat.com>>
>  ---
>  hw/net/virtio-net.c | 29
>  +
>  include/hw/virtio/virtio-net.h  |  3 +++
>  include/standard-headers/linux/virtio_net.h |  4 
>  3 files changed, 36 insertions(+)
> 
>  diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
>  index 38674b0..d63e790 100644
>  --- a/hw/net/virtio-net.c
>  +++ b/hw/net/virtio-net.c
>  @@ -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 defines */
>  +#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 = 1 << VIRTIO_NET_F_MTU,
>  .end = endof(struct virtio_net_config, mtu)},
>  +    {.flags = 1 << VIRTIO_NET_F_SPEED_DUPLEX,
>  + .end = endof(struct virtio_net_config, duplex)},
>     {}
>  };
> 
>  @@ -88,6 +96,8 @@ static void virtio_net_get_config(VirtIODevice
>  *vdev, uint8_t *config)
>     virtio_stw_p(vdev, &netcfg.status, n->status);
>     virtio_stw_p(vdev, &netcfg.max_virtqueue_pairs, n->max_queues);
>     virtio_stw_p(vdev, &netcfg.mtu, n->net_conf.mtu);
>  +    virtio_stl_p(vdev, &netcfg.speed, n->net_conf.speed);
>  +    netcfg.duplex = n->net_conf.duplex;
>     memcpy(netcfg.mac, n->mac, ETH_ALEN);
>     memcpy(config, &netcfg, n->config_size);
>  }
>  @@ -1941,6 +1951,23 @@ static void
>  virtio_net_device_realize(DeviceState *dev, Error **errp)
>     n->host_features |= (0x1 << VIRTIO_NET_F_MTU);
>     }
> 
>  +    n->host_features |= (0x1 << 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 +2187,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),
> >>>
> >>> From Windows guest perspective I prefer to have some reasonable
> >>> default (10G for example). 
> >>
> >>
> >> hmmm, I didn't want to change/set the default here in case it broke
> >> something, but I'm ok setting it to some 'reasonable' value - (10G and
> >> duplex?), if the consensus is that that would be safe.
> > 
> > OK from my side.
> > Thanks.
> 
> I presume your speaking for windows - i'm wondering if under linux the
> virtio device suddenly start showing up as duplex, 10Gbps, will that
> break anything? I can speak for the use-cases we have here and that
> would certainly be fine for us, but i

Re: [Qemu-devel] [PATCH 2/2] qemu: add linkspeed and duplex setting to virtio-net

2017-12-19 Thread Jason Baron via Qemu-devel 


On 12/19/2017 04:19 AM, Yan Vugenfirer wrote:
> 
>> On 18 Dec 2017, at 18:04, Jason Baron via Qemu-devel
>> mailto:qemu-devel@nongnu.org>> wrote:
>>
>>
>>
>> On 12/18/2017 06:34 AM, Yan Vugenfirer wrote:
>>>
 On 14 Dec 2017, at 21:33, Jason Baron via Qemu-devel
 mailto:qemu-devel@nongnu.org>> wrote:

 Although they can be currently set in linux via 'ethtool -s', this
 requires
 guest changes, and thus it would be nice to extend this
 functionality such
 that it can be configured automatically from the host (as other network
 do).

 Linkspeed and duplex settings can be set as:
 '-device virtio-net,speed=1,duplex=full'

 where speed is [-1...INT_MAX], and duplex is ["half"|"full"].

 Signed-off-by: Jason Baron >>> >
 Cc: "Michael S. Tsirkin" mailto:m...@redhat.com>>
 Cc: Jason Wang mailto:jasow...@redhat.com>>
 ---
 hw/net/virtio-net.c | 29
 +
 include/hw/virtio/virtio-net.h  |  3 +++
 include/standard-headers/linux/virtio_net.h |  4 
 3 files changed, 36 insertions(+)

 diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
 index 38674b0..d63e790 100644
 --- a/hw/net/virtio-net.c
 +++ b/hw/net/virtio-net.c
 @@ -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 defines */
 +#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 = 1 << VIRTIO_NET_F_MTU,
 .end = endof(struct virtio_net_config, mtu)},
 +    {.flags = 1 << VIRTIO_NET_F_SPEED_DUPLEX,
 + .end = endof(struct virtio_net_config, duplex)},
    {}
 };

 @@ -88,6 +96,8 @@ static void virtio_net_get_config(VirtIODevice
 *vdev, uint8_t *config)
    virtio_stw_p(vdev, &netcfg.status, n->status);
    virtio_stw_p(vdev, &netcfg.max_virtqueue_pairs, n->max_queues);
    virtio_stw_p(vdev, &netcfg.mtu, n->net_conf.mtu);
 +    virtio_stl_p(vdev, &netcfg.speed, n->net_conf.speed);
 +    netcfg.duplex = n->net_conf.duplex;
    memcpy(netcfg.mac, n->mac, ETH_ALEN);
    memcpy(config, &netcfg, n->config_size);
 }
 @@ -1941,6 +1951,23 @@ static void
 virtio_net_device_realize(DeviceState *dev, Error **errp)
    n->host_features |= (0x1 << VIRTIO_NET_F_MTU);
    }

 +    n->host_features |= (0x1 << 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 +2187,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),
>>>
>>> From Windows guest perspective I prefer to have some reasonable
>>> default (10G for example). 
>>
>>
>> hmmm, I didn't want to change/set the default here in case it broke
>> something, but I'm ok setting it to some 'reasonable' value - (10G and
>> duplex?), if the consensus is that that would be safe.
> 
> OK from my side.
> Thanks.

I presume your speaking for windows - i'm wondering if under linux the
virtio device suddenly start showing up as duplex, 10Gbps, will that
break anything? I can speak for the use-cases we have here and that
would certainly be fine for us, but i'm really not sure if that is ok
more generally.

Thanks,

-Jason

> 
>>
>> Thanks,
>>
>> -Jason
>>
>>>
>>> Thanks,
>>> Yan.
>>>
 +    DEFINE_PROP_STRING("duplex", VirtIONet, net_conf.duplex_str),
    DEFINE_PROP_END_OF_LIST(),
 };

 diff --git a/include/hw/virtio/virtio-net.h
 b/

Re: [Qemu-devel] [PATCH 2/2] qemu: add linkspeed and duplex setting to virtio-net

2017-12-19 Thread Yan Vugenfirer 

> On 18 Dec 2017, at 18:04, Jason Baron via Qemu-devel  
> wrote:
> 
> 
> 
> On 12/18/2017 06:34 AM, Yan Vugenfirer wrote:
>> 
>>> On 14 Dec 2017, at 21:33, Jason Baron via Qemu-devel 
>>>  wrote:
>>> 
>>> Although they can be currently set in linux via 'ethtool -s', this requires
>>> guest changes, and thus it would be nice to extend this functionality such
>>> that it can be configured automatically from the host (as other network
>>> do).
>>> 
>>> Linkspeed and duplex settings can be set as:
>>> '-device virtio-net,speed=1,duplex=full'
>>> 
>>> where speed is [-1...INT_MAX], and duplex is ["half"|"full"].
>>> 
>>> Signed-off-by: Jason Baron 
>>> Cc: "Michael S. Tsirkin" 
>>> Cc: Jason Wang 
>>> ---
>>> hw/net/virtio-net.c | 29 
>>> +
>>> include/hw/virtio/virtio-net.h  |  3 +++
>>> include/standard-headers/linux/virtio_net.h |  4 
>>> 3 files changed, 36 insertions(+)
>>> 
>>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
>>> index 38674b0..d63e790 100644
>>> --- a/hw/net/virtio-net.c
>>> +++ b/hw/net/virtio-net.c
>>> @@ -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 defines */
>>> +#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 = 1 << VIRTIO_NET_F_MTU,
>>> .end = endof(struct virtio_net_config, mtu)},
>>> +{.flags = 1 << VIRTIO_NET_F_SPEED_DUPLEX,
>>> + .end = endof(struct virtio_net_config, duplex)},
>>>{}
>>> };
>>> 
>>> @@ -88,6 +96,8 @@ static void virtio_net_get_config(VirtIODevice *vdev, 
>>> uint8_t *config)
>>>virtio_stw_p(vdev, &netcfg.status, n->status);
>>>virtio_stw_p(vdev, &netcfg.max_virtqueue_pairs, n->max_queues);
>>>virtio_stw_p(vdev, &netcfg.mtu, n->net_conf.mtu);
>>> +virtio_stl_p(vdev, &netcfg.speed, n->net_conf.speed);
>>> +netcfg.duplex = n->net_conf.duplex;
>>>memcpy(netcfg.mac, n->mac, ETH_ALEN);
>>>memcpy(config, &netcfg, n->config_size);
>>> }
>>> @@ -1941,6 +1951,23 @@ static void virtio_net_device_realize(DeviceState 
>>> *dev, Error **errp)
>>>n->host_features |= (0x1 << VIRTIO_NET_F_MTU);
>>>}
>>> 
>>> +n->host_features |= (0x1 << 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 +2187,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),
>> 
>> From Windows guest perspective I prefer to have some reasonable default (10G 
>> for example). 
> 
> 
> hmmm, I didn't want to change/set the default here in case it broke
> something, but I'm ok setting it to some 'reasonable' value - (10G and
> duplex?), if the consensus is that that would be safe.

OK from my side.
Thanks.

> 
> Thanks,
> 
> -Jason
> 
>> 
>> Thanks,
>> Yan.
>> 
>>> +DEFINE_PROP_STRING("duplex", VirtIONet, net_conf.duplex_str),
>>>DEFINE_PROP_END_OF_LIST(),
>>> };
>>> 
>>> diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h
>>> index b81b6a4..af74a94 100644
>>> --- a/include/hw/virtio/virtio-net.h
>>> +++ b/include/hw/virtio/virtio-net.h
>>> @@ -38,6 +38,9 @@ typedef struct virtio_net_conf
>>>uint16_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 */
>>> diff --git a/include/standard-headers/linux/virtio_net.h 
>>> b/include/standard-headers/linux/virtio_net.h
>>> index 30ff249..0ff1447 100644
>>> --- a/include/sta

Re: [Qemu-devel] [PATCH 2/2] qemu: add linkspeed and duplex setting to virtio-net

2017-12-18 Thread Jason Baron via Qemu-devel 


On 12/18/2017 06:34 AM, Yan Vugenfirer wrote:
> 
>> On 14 Dec 2017, at 21:33, Jason Baron via Qemu-devel  
>> wrote:
>>
>> Although they can be currently set in linux via 'ethtool -s', this requires
>> guest changes, and thus it would be nice to extend this functionality such
>> that it can be configured automatically from the host (as other network
>> do).
>>
>> Linkspeed and duplex settings can be set as:
>> '-device virtio-net,speed=1,duplex=full'
>>
>> where speed is [-1...INT_MAX], and duplex is ["half"|"full"].
>>
>> Signed-off-by: Jason Baron 
>> Cc: "Michael S. Tsirkin" 
>> Cc: Jason Wang 
>> ---
>> hw/net/virtio-net.c | 29 
>> +
>> include/hw/virtio/virtio-net.h  |  3 +++
>> include/standard-headers/linux/virtio_net.h |  4 
>> 3 files changed, 36 insertions(+)
>>
>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
>> index 38674b0..d63e790 100644
>> --- a/hw/net/virtio-net.c
>> +++ b/hw/net/virtio-net.c
>> @@ -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 defines */
>> +#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 = 1 << VIRTIO_NET_F_MTU,
>>  .end = endof(struct virtio_net_config, mtu)},
>> +{.flags = 1 << VIRTIO_NET_F_SPEED_DUPLEX,
>> + .end = endof(struct virtio_net_config, duplex)},
>> {}
>> };
>>
>> @@ -88,6 +96,8 @@ static void virtio_net_get_config(VirtIODevice *vdev, 
>> uint8_t *config)
>> virtio_stw_p(vdev, &netcfg.status, n->status);
>> virtio_stw_p(vdev, &netcfg.max_virtqueue_pairs, n->max_queues);
>> virtio_stw_p(vdev, &netcfg.mtu, n->net_conf.mtu);
>> +virtio_stl_p(vdev, &netcfg.speed, n->net_conf.speed);
>> +netcfg.duplex = n->net_conf.duplex;
>> memcpy(netcfg.mac, n->mac, ETH_ALEN);
>> memcpy(config, &netcfg, n->config_size);
>> }
>> @@ -1941,6 +1951,23 @@ static void virtio_net_device_realize(DeviceState 
>> *dev, Error **errp)
>> n->host_features |= (0x1 << VIRTIO_NET_F_MTU);
>> }
>>
>> +n->host_features |= (0x1 << 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 +2187,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),
> 
> From Windows guest perspective I prefer to have some reasonable default (10G 
> for example). 


hmmm, I didn't want to change/set the default here in case it broke
something, but I'm ok setting it to some 'reasonable' value - (10G and
duplex?), if the consensus is that that would be safe.

Thanks,

-Jason

> 
> Thanks,
> Yan.
> 
>> +DEFINE_PROP_STRING("duplex", VirtIONet, net_conf.duplex_str),
>> DEFINE_PROP_END_OF_LIST(),
>> };
>>
>> diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h
>> index b81b6a4..af74a94 100644
>> --- a/include/hw/virtio/virtio-net.h
>> +++ b/include/hw/virtio/virtio-net.h
>> @@ -38,6 +38,9 @@ typedef struct virtio_net_conf
>> uint16_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 */
>> diff --git a/include/standard-headers/linux/virtio_net.h 
>> b/include/standard-headers/linux/virtio_net.h
>> index 30ff249..0ff1447 100644
>> --- a/include/standard-headers/linux/virtio_net.h
>> +++ b/include/standard-headers/linux/virtio_net.h
>> @@ -36,6 +36,7 @@
>> #define VIRTIO_NET_F_GUEST_CSUM  1   /* Guest handles pkts w/ 
>> partial csum */
>> #define VIRTIO_NET_F_CTRL_GUE

Re: [Qemu-devel] [PATCH 2/2] qemu: add linkspeed and duplex setting to virtio-net

2017-12-18 Thread Yan Vugenfirer 

> On 14 Dec 2017, at 21:33, Jason Baron via Qemu-devel  
> wrote:
> 
> Although they can be currently set in linux via 'ethtool -s', this requires
> guest changes, and thus it would be nice to extend this functionality such
> that it can be configured automatically from the host (as other network
> do).
> 
> Linkspeed and duplex settings can be set as:
> '-device virtio-net,speed=1,duplex=full'
> 
> where speed is [-1...INT_MAX], and duplex is ["half"|"full"].
> 
> Signed-off-by: Jason Baron 
> Cc: "Michael S. Tsirkin" 
> Cc: Jason Wang 
> ---
> hw/net/virtio-net.c | 29 +
> include/hw/virtio/virtio-net.h  |  3 +++
> include/standard-headers/linux/virtio_net.h |  4 
> 3 files changed, 36 insertions(+)
> 
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index 38674b0..d63e790 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -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 defines */
> +#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 = 1 << VIRTIO_NET_F_MTU,
>  .end = endof(struct virtio_net_config, mtu)},
> +{.flags = 1 << VIRTIO_NET_F_SPEED_DUPLEX,
> + .end = endof(struct virtio_net_config, duplex)},
> {}
> };
> 
> @@ -88,6 +96,8 @@ static void virtio_net_get_config(VirtIODevice *vdev, 
> uint8_t *config)
> virtio_stw_p(vdev, &netcfg.status, n->status);
> virtio_stw_p(vdev, &netcfg.max_virtqueue_pairs, n->max_queues);
> virtio_stw_p(vdev, &netcfg.mtu, n->net_conf.mtu);
> +virtio_stl_p(vdev, &netcfg.speed, n->net_conf.speed);
> +netcfg.duplex = n->net_conf.duplex;
> memcpy(netcfg.mac, n->mac, ETH_ALEN);
> memcpy(config, &netcfg, n->config_size);
> }
> @@ -1941,6 +1951,23 @@ static void virtio_net_device_realize(DeviceState 
> *dev, Error **errp)
> n->host_features |= (0x1 << VIRTIO_NET_F_MTU);
> }
> 
> +n->host_features |= (0x1 << 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 +2187,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),

From Windows guest perspective I prefer to have some reasonable default (10G 
for example). 

Thanks,
Yan.

> +DEFINE_PROP_STRING("duplex", VirtIONet, net_conf.duplex_str),
> DEFINE_PROP_END_OF_LIST(),
> };
> 
> diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h
> index b81b6a4..af74a94 100644
> --- a/include/hw/virtio/virtio-net.h
> +++ b/include/hw/virtio/virtio-net.h
> @@ -38,6 +38,9 @@ typedef struct virtio_net_conf
> uint16_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 */
> diff --git a/include/standard-headers/linux/virtio_net.h 
> b/include/standard-headers/linux/virtio_net.h
> index 30ff249..0ff1447 100644
> --- a/include/standard-headers/linux/virtio_net.h
> +++ b/include/standard-headers/linux/virtio_net.h
> @@ -36,6 +36,7 @@
> #define VIRTIO_NET_F_GUEST_CSUM   1   /* Guest handles pkts w/ 
> partial csum */
> #define VIRTIO_NET_F_CTRL_GUEST_OFFLOADS 2 /* Dynamic offload configuration. 
> */
> #define VIRTIO_NET_F_MTU  3   /* Initial MTU advice */
> +#define VIRTIO_NET_F_SPEED_DUPLEX 4  /* Host set linkspeed and duplex */
> #define VIRTIO_NET_F_MAC  5   /* Host has given MAC address. */
> #define VIRTIO_NET_F_GUEST_TSO4   7   /* Guest can handle TSOv4 in. */
> #define VIRTIO_NET_F_GUEST_TSO6