| Message ID | c9d3cf2fbc3c64bb28e0438185d85d95b2088127.1519961667.git.jbaron@akamai.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 2018年03月02日 11:46, Jason Baron wrote: > Although 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' I was thinking whether or not it's better to decide the duplex by the type of backends. E.g userspace and vhost-kernel implement a in fact half duplex. But dpdk implement a full duplex. Thanks > > where speed is [0...INT_MAX], and duplex is ["half"|"full"]. > > Signed-off-by: Jason Baron<jbaron@akamai.com> > Cc: "Michael S. Tsirkin"<mst@redhat.com> > Cc: Jason Wang<jasowang@redhat.com> > Cc:virtio-dev@lists.oasis-open.org > ---
On 03/02/2018 02:14 AM, Jason Wang wrote: > > > On 2018年03月02日 11:46, Jason Baron wrote: >> Although 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' > > I was thinking whether or not it's better to decide the duplex by the > type of backends. > > E.g userspace and vhost-kernel implement a in fact half duplex. But dpdk > implement a full duplex. Interesting - could this be derived only from the backend 'type'. IE: NET_CLIENT_DRIVER_TAP, NET_CLIENT_DRIVER_VHOST_USER... I was also thinking this could be specified as 'duplex=backend', in addition to the proposed 'duplex=full' or 'duplex=half'? Thanks, -Jason > > Thanks > >> >> where speed is [0...INT_MAX], and duplex is ["half"|"full"]. >> >> Signed-off-by: Jason Baron<jbaron@akamai.com> >> Cc: "Michael S. Tsirkin"<mst@redhat.com> >> Cc: Jason Wang<jasowang@redhat.com> >> Cc:virtio-dev@lists.oasis-open.org >> --- >
On Fri, Mar 02, 2018 at 03:14:01PM +0800, Jason Wang wrote: > > > On 2018年03月02日 11:46, Jason Baron wrote: > > Although 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' > > I was thinking whether or not it's better to decide the duplex by the type > of backends. > > E.g userspace and vhost-kernel implement a in fact half duplex. But dpdk > implement a full duplex. > > Thanks OTOH it's a priority for some people to be able to support migration between different backend types. Breaking that won't be nice. > > > > where speed is [0...INT_MAX], and duplex is ["half"|"full"]. > > > > Signed-off-by: Jason Baron<jbaron@akamai.com> > > Cc: "Michael S. Tsirkin"<mst@redhat.com> > > Cc: Jason Wang<jasowang@redhat.com> > > Cc:virtio-dev@lists.oasis-open.org > > ---
On Fri, Mar 02, 2018 at 11:59:00AM -0500, Jason Baron wrote: > On 03/02/2018 02:14 AM, Jason Wang wrote: > > > > > > On 2018年03月02日 11:46, Jason Baron wrote: > >> Although 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' > > > > I was thinking whether or not it's better to decide the duplex by the > > type of backends. > > > > E.g userspace and vhost-kernel implement a in fact half duplex. But dpdk > > implement a full duplex. > > Interesting - could this be derived only from the backend 'type'. IE: > NET_CLIENT_DRIVER_TAP, NET_CLIENT_DRIVER_VHOST_USER... > > > I was also thinking this could be specified as 'duplex=backend', in > addition to the proposed 'duplex=full' or 'duplex=half'? > > Thanks, > > -Jason I'd say it would make more sense to teach backends to obey what's specified by the user. E.g. if vhost gets a duplex config, create two threads. But I think all that's for future, we can just fake it for now - the current uses don't seem to particularly care about whether virtio actually is or isn't a duplex. > > > > Thanks > > > >> > >> where speed is [0...INT_MAX], and duplex is ["half"|"full"]. > >> > >> Signed-off-by: Jason Baron<jbaron@akamai.com> > >> Cc: "Michael S. Tsirkin"<mst@redhat.com> > >> Cc: Jason Wang<jasowang@redhat.com> > >> Cc:virtio-dev@lists.oasis-open.org > >> --- > >
> On 2 Mar 2018, at 22:19, Michael S. Tsirkin <mst@redhat.com> wrote: > > On Fri, Mar 02, 2018 at 03:14:01PM +0800, Jason Wang wrote: >> >> >> On 2018年03月02日 11:46, Jason Baron wrote: >>> Although 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' >> >> I was thinking whether or not it's better to decide the duplex by the type >> of backends. >> >> E.g userspace and vhost-kernel implement a in fact half duplex. But dpdk >> implement a full duplex. >> >> Thanks > > OTOH it's a priority for some people to be able to support migration > between different backend types. Breaking that won't be nice. I think that in this case we need a way to update the settings of link speed and link duplex (maybe add QMP command). Migration between different backend types should cause link down\link up events. And this is a time for a driver to re-read the settings and update the OS. Best regards, Yan. > >>> >>> where speed is [0...INT_MAX], and duplex is ["half"|"full"]. >>> >>> Signed-off-by: Jason Baron<jbaron@akamai.com> >>> Cc: "Michael S. Tsirkin"<mst@redhat.com> >>> Cc: Jason Wang<jasowang@redhat.com> >>> Cc:virtio-dev@lists.oasis-open.org >>> --- > > --------------------------------------------------------------------- > To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org > For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
On 03/02/2018 03:22 PM, Michael S. Tsirkin wrote: > On Fri, Mar 02, 2018 at 11:59:00AM -0500, Jason Baron wrote: >> On 03/02/2018 02:14 AM, Jason Wang wrote: >>> >>> >>> On 2018年03月02日 11:46, Jason Baron wrote: >>>> Although 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' >>> >>> I was thinking whether or not it's better to decide the duplex by the >>> type of backends. >>> >>> E.g userspace and vhost-kernel implement a in fact half duplex. But dpdk >>> implement a full duplex. >> >> Interesting - could this be derived only from the backend 'type'. IE: >> NET_CLIENT_DRIVER_TAP, NET_CLIENT_DRIVER_VHOST_USER... >> >> >> I was also thinking this could be specified as 'duplex=backend', in >> addition to the proposed 'duplex=full' or 'duplex=half'? >> >> Thanks, >> >> -Jason > > I'd say it would make more sense to teach backends to obey what's > specified by the user. E.g. if vhost gets a duplex config, > create two threads. > > But I think all that's for future, we can just fake it for > now - the current uses don't seem to particularly care about whether > virtio actually is or isn't a duplex. > > Ok, I wouldn't add 'duplex=backend' when I re-post, and will leave any automatic settings of duplex to 'future work'. Thanks, -Jason
On 03/04/2018 08:05 AM, Yan Vugenfirer wrote: > > >> On 2 Mar 2018, at 22:19, Michael S. Tsirkin <mst@redhat.com >> <mailto:mst@redhat.com>> wrote: >> >> On Fri, Mar 02, 2018 at 03:14:01PM +0800, Jason Wang wrote: >>> >>> >>> On 2018年03月02日 11:46, Jason Baron wrote: >>>> Although 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' >>> >>> I was thinking whether or not it's better to decide the duplex by the >>> type >>> of backends. >>> >>> E.g userspace and vhost-kernel implement a in fact half duplex. But dpdk >>> implement a full duplex. >>> >>> Thanks >> >> OTOH it's a priority for some people to be able to support migration >> between different backend types. Breaking that won't be nice. > > I think that in this case we need a way to update the settings of link > speed and link duplex (maybe add QMP command). Migration between > different backend types should cause link down\link up events. And this > is a time for a driver to re-read the settings and update the OS. > > Best regards, > Yan. > So the virtio_net driver in linux will re-read these settings on link up events. So I could add a qmp command to set these (in addition to the command-line) interface, if desired. Is there a consensus that we need to add a qmp command here? Or can that be treated as a future item, if somebody wants it? Thanks, -Jason
On Tue, Mar 06, 2018 at 01:02:06PM -0500, Jason Baron wrote: > > > On 03/04/2018 08:05 AM, Yan Vugenfirer wrote: > > > > > >> On 2 Mar 2018, at 22:19, Michael S. Tsirkin <mst@redhat.com > >> <mailto:mst@redhat.com>> wrote: > >> > >> On Fri, Mar 02, 2018 at 03:14:01PM +0800, Jason Wang wrote: > >>> > >>> > >>> On 2018年03月02日 11:46, Jason Baron wrote: > >>>> Although 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' > >>> > >>> I was thinking whether or not it's better to decide the duplex by the > >>> type > >>> of backends. > >>> > >>> E.g userspace and vhost-kernel implement a in fact half duplex. But dpdk > >>> implement a full duplex. > >>> > >>> Thanks > >> > >> OTOH it's a priority for some people to be able to support migration > >> between different backend types. Breaking that won't be nice. > > > > I think that in this case we need a way to update the settings of link > > speed and link duplex (maybe add QMP command). Migration between > > different backend types should cause link down\link up events. And this > > is a time for a driver to re-read the settings and update the OS. > > > > Best regards, > > Yan. > > > > So the virtio_net driver in linux will re-read these settings on link up > events. So I could add a qmp command to set these (in addition to the > command-line) interface, if desired. Is there a consensus that we need > to add a qmp command here? Or can that be treated as a future item, if > somebody wants it? > > Thanks, > > -Jason We already have ability to take link down and up. I'd say it's a future item.
> On 6 Mar 2018, at 20:13, Michael S. Tsirkin <mst@redhat.com> wrote: > > On Tue, Mar 06, 2018 at 01:02:06PM -0500, Jason Baron wrote: >> >> >> On 03/04/2018 08:05 AM, Yan Vugenfirer wrote: >>> >>> >>>> On 2 Mar 2018, at 22:19, Michael S. Tsirkin <mst@redhat.com >>>> <mailto:mst@redhat.com>> wrote: >>>> >>>> On Fri, Mar 02, 2018 at 03:14:01PM +0800, Jason Wang wrote: >>>>> >>>>> >>>>> On 2018年03月02日 11:46, Jason Baron wrote: >>>>>> Although 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' >>>>> >>>>> I was thinking whether or not it's better to decide the duplex by the >>>>> type >>>>> of backends. >>>>> >>>>> E.g userspace and vhost-kernel implement a in fact half duplex. But dpdk >>>>> implement a full duplex. >>>>> >>>>> Thanks >>>> >>>> OTOH it's a priority for some people to be able to support migration >>>> between different backend types. Breaking that won't be nice. >>> >>> I think that in this case we need a way to update the settings of link >>> speed and link duplex (maybe add QMP command). Migration between >>> different backend types should cause link down\link up events. And this >>> is a time for a driver to re-read the settings and update the OS. >>> >>> Best regards, >>> Yan. >>> >> >> So the virtio_net driver in linux will re-read these settings on link up >> events. So I could add a qmp command to set these (in addition to the >> command-line) interface, if desired. Is there a consensus that we need >> to add a qmp command here? Or can that be treated as a future item, if >> somebody wants it? >> >> Thanks, >> >> -Jason > > We already have ability to take link down and up. > I'd say it's a future item. I agree. Yan. > > -- > MST
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index 4feaa49..5df90ea 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -26,6 +26,7 @@ #include "qapi-event.h" #include "hw/virtio/virtio-access.h" #include "migration/misc.h" +#include "net/eth.h" #define VIRTIO_NET_VM_VERSION 11 @@ -61,6 +62,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 +92,8 @@ 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); + virtio_stl_p(vdev, &netcfg.speed, n->net_conf.speed); + netcfg.duplex = n->net_conf.duplex; memcpy(config, &netcfg, n->config_size); } @@ -1941,6 +1946,25 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp) n->host_features |= (1ULL << VIRTIO_NET_F_MTU); } + 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'"); + } + n->host_features |= (1ULL << VIRTIO_NET_F_SPEED_DUPLEX); + } else { + n->net_conf.duplex = DUPLEX_UNKNOWN; + } + + if (n->net_conf.speed < SPEED_UNKNOWN) { + error_setg(errp, "'speed' must be between 0 and INT_MAX"); + } else if (n->net_conf.speed >= 0) { + n->host_features |= (1ULL << VIRTIO_NET_F_SPEED_DUPLEX); + } + virtio_net_set_config_size(n, n->host_features); virtio_init(vdev, "virtio-net", VIRTIO_ID_NET, n->config_size); @@ -2161,6 +2185,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(), }; diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h index e7634c9..02484dc 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 */
Although 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 [0...INT_MAX], and duplex is ["half"|"full"]. Signed-off-by: Jason Baron <jbaron@akamai.com> Cc: "Michael S. Tsirkin" <mst@redhat.com> Cc: Jason Wang <jasowang@redhat.com> Cc: virtio-dev@lists.oasis-open.org --- hw/net/virtio-net.c | 26 ++++++++++++++++++++++++++ include/hw/virtio/virtio-net.h | 3 +++ 2 files changed, 29 insertions(+)