Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752744AbdLSQwy (ORCPT ); Tue, 19 Dec 2017 11:52:54 -0500 Received: from mx0b-00190b01.pphosted.com ([67.231.157.127]:36208 "EHLO mx0b-00190b01.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751798AbdLSQwq (ORCPT ); Tue, 19 Dec 2017 11:52:46 -0500 Subject: Re: [Qemu-devel] [PATCH 2/2] qemu: add linkspeed and duplex setting to virtio-net To: Yan Vugenfirer Cc: jasowang@redhat.com, mst@redhat.com, qemu-devel@nongnu.org, linux-kernel@vger.kernel.org References: <1513280073-27515-1-git-send-email-jbaron@akamai.com> <9d600f3d-7e8d-153f-fd34-704c9975099b@akamai.com> <21A64922-D22E-4AE1-8B4C-B0D85357240C@redhat.com> From: Jason Baron Message-ID: <73e8811d-70fd-4920-1cb4-ad1809ebff82@akamai.com> Date: Tue, 19 Dec 2017 11:52:39 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: <21A64922-D22E-4AE1-8B4C-B0D85357240C@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Language: en-US Content-Transfer-Encoding: 8bit X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-12-19_09:,, signatures=0 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 suspectscore=0 malwarescore=0 phishscore=0 bulkscore=0 spamscore=0 mlxscore=0 mlxlogscore=999 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1711220000 definitions=main-1712190240 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-12-19_09:,, signatures=0 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 priorityscore=1501 malwarescore=0 suspectscore=0 phishscore=0 bulkscore=0 spamscore=0 clxscore=1015 lowpriorityscore=0 mlxscore=0 impostorscore=0 mlxlogscore=999 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1711220000 definitions=main-1712190240 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6663 Lines: 184 On 12/19/2017 04:19 AM, Yan Vugenfirer wrote: > >> 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=10000,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. 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/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_CSUM1/* Guest handles pkts w/ partial csum */ >>>> #define VIRTIO_NET_F_CTRL_GUEST_OFFLOADS 2 /* Dynamic offload >>>> configuration. */ >>>> #define VIRTIO_NET_F_MTU3/* Initial MTU advice */ >>>> +#define VIRTIO_NET_F_SPEED_DUPLEX 4/* Host set linkspeed and duplex */ >>>> #define VIRTIO_NET_F_MAC5/* Host has given MAC address. */ >>>> #define VIRTIO_NET_F_GUEST_TSO47/* Guest can handle TSOv4 in. */ >>>> #define VIRTIO_NET_F_GUEST_TSO68/* Guest can handle TSOv6 in. */ >>>> @@ -76,6 +77,9 @@ struct virtio_net_config { >>>> uint16_t max_virtqueue_pairs; >>>> /* Default maximum transmit unit advice */ >>>> uint16_t mtu; >>>> +/* Host exported linkspeed and duplex */ >>>> +uint32_t speed; >>>> +uint8_t duplex; >>>> } QEMU_PACKED; >>>> >>>> /* >>>> --? >>>> 2.6.1 >