[virtio-dev] [PATCH 1/4] eth: add speed and duplex definitions

[virtio-dev] [PATCH 1/4] eth: add speed and duplex definitions

[Jason Baron]

Pull in definitions for SPEED_UNKNOWN, DUPLEX_UNKNOWN, DUPLEX_HALF,
and DUPLEX_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
---
 include/net/eth.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/include/net/eth.h b/include/net/eth.h
index 09054a5..9843678 100644
--- a/include/net/eth.h
+++ b/include/net/eth.h
@@ -417,4 +417,11 @@ bool
 eth_parse_ipv6_hdr(const struct iovec *pkt, int pkt_frags,
                    size_t ip6hdr_off, eth_ip6_hdr_info *info);
 
+/* ethtool defines - from linux/ethtool.h */
+#define SPEED_UNKNOWN           -1
+
+#define DUPLEX_HALF             0x00
+#define DUPLEX_FULL             0x01
+#define DUPLEX_UNKNOWN          0xff
+
 #endif
-- 
2.7.4


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] [PATCH 2/4] rocker: drop local duplex definitions

[Jason Baron]

Make use of duplex definitions from net/eth.h.

Signed-off-by: Jason Baron <jbaron@akamai.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Jason Wang <jasowang@redhat.com>
Cc: Jiri Pirko <jiri@resnulli.us>
Cc: virtio-dev@lists.oasis-open.org
---
 hw/net/rocker/rocker_fp.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/hw/net/rocker/rocker_fp.c b/hw/net/rocker/rocker_fp.c
index 4b3c984..13a14a0 100644
--- a/hw/net/rocker/rocker_fp.c
+++ b/hw/net/rocker/rocker_fp.c
@@ -16,17 +16,13 @@
 
 #include "qemu/osdep.h"
 #include "net/clients.h"
+#include "net/eth.h"
 
 #include "rocker.h"
 #include "rocker_hw.h"
 #include "rocker_fp.h"
 #include "rocker_world.h"
 
-enum duplex {
-    DUPLEX_HALF = 0,
-    DUPLEX_FULL
-};
-
 struct fp_port {
     Rocker *r;
     World *world;
-- 
2.7.4


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] [PATCH 3/4] virtio-net: use 64-bit values for feature flags

[Jason Baron]

In prepartion for using some of the high order feature bits, make sure that
virtio-net uses 64-bit values everywhere.

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            | 55 +++++++++++++++++++++---------------------
 include/hw/virtio/virtio-net.h |  2 +-
 2 files changed, 29 insertions(+), 28 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 369d40b..4feaa49 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -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,46 @@ 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),
diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h
index b81b6a4..e7634c9 100644
--- a/include/hw/virtio/virtio-net.h
+++ b/include/hw/virtio/virtio-net.h
@@ -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;
-- 
2.7.4


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] [PATCH 4/4] virtio-net: add linkspeed and duplex settings to virtio-net

[Jason Baron]

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(+)

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 */
-- 
2.7.4


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] Re: [PATCH 4/4] virtio-net: add linkspeed and duplex settings to virtio-net

[Jason Wang]

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
> ---


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] Re: [PATCH 4/4] virtio-net: add linkspeed and duplex settings to virtio-net

[Jason Baron]

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
>> ---
> 

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] Re: [PATCH 1/4] eth: add speed and duplex definitions

[Michael S. Tsirkin]

On Thu, Mar 01, 2018 at 10:46:33PM -0500, Jason Baron wrote:
> Pull in definitions for SPEED_UNKNOWN, DUPLEX_UNKNOWN, DUPLEX_HALF,
> and DUPLEX_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
> ---
>  include/net/eth.h | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/include/net/eth.h b/include/net/eth.h
> index 09054a5..9843678 100644
> --- a/include/net/eth.h
> +++ b/include/net/eth.h
> @@ -417,4 +417,11 @@ bool
>  eth_parse_ipv6_hdr(const struct iovec *pkt, int pkt_frags,
>                     size_t ip6hdr_off, eth_ip6_hdr_info *info);
>  
> +/* ethtool defines - from linux/ethtool.h */
> +#define SPEED_UNKNOWN           -1
> +
> +#define DUPLEX_HALF             0x00
> +#define DUPLEX_FULL             0x01
> +#define DUPLEX_UNKNOWN          0xff
> +
>  #endif

While that's not a lot, I think we should import linux/ethtool.h into
include/standard-headers/linux/ using scripts/update-linux-headers.sh

> -- 
> 2.7.4

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] Re: [PATCH 2/4] rocker: drop local duplex definitions

[Michael S. Tsirkin]

On Thu, Mar 01, 2018 at 10:46:34PM -0500, Jason Baron wrote:
> Make use of duplex definitions from net/eth.h.
> 
> Signed-off-by: Jason Baron <jbaron@akamai.com>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Jason Wang <jasowang@redhat.com>
> Cc: Jiri Pirko <jiri@resnulli.us>
> Cc: virtio-dev@lists.oasis-open.org
> ---
>  hw/net/rocker/rocker_fp.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/hw/net/rocker/rocker_fp.c b/hw/net/rocker/rocker_fp.c
> index 4b3c984..13a14a0 100644
> --- a/hw/net/rocker/rocker_fp.c
> +++ b/hw/net/rocker/rocker_fp.c
> @@ -16,17 +16,13 @@
>  
>  #include "qemu/osdep.h"
>  #include "net/clients.h"
> +#include "net/eth.h"
>  
>  #include "rocker.h"
>  #include "rocker_hw.h"
>  #include "rocker_fp.h"
>  #include "rocker_world.h"
>  
> -enum duplex {
> -    DUPLEX_HALF = 0,
> -    DUPLEX_FULL
> -};
> -

This is hardware emulation, so I actually suspect it
should be renamed ROCKER_FP_HALF/ROCKER_FP_FULL.

>  struct fp_port {
>      Rocker *r;
>      World *world;
> -- 
> 2.7.4

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] Re: [PATCH 4/4] virtio-net: add linkspeed and duplex settings to virtio-net

[Michael S. Tsirkin]

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
> > ---

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] Re: [PATCH 4/4] virtio-net: add linkspeed and duplex settings to virtio-net

[Michael S. Tsirkin]

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
> >> ---
> > 

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

Re: [virtio-dev] [PATCH 4/4] virtio-net: add linkspeed and duplex settings to virtio-net

[Yan Vugenfirer]

[-- Attachment #1: Type: text/plain, Size: 1846 bytes --]



> 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


[-- Attachment #2: Type: text/html, Size: 8094 bytes --]

[virtio-dev] Re: [PATCH 1/4] eth: add speed and duplex definitions

[Jason Baron]

On 03/02/2018 12:54 PM, Michael S. Tsirkin wrote:
> On Thu, Mar 01, 2018 at 10:46:33PM -0500, Jason Baron wrote:
>> Pull in definitions for SPEED_UNKNOWN, DUPLEX_UNKNOWN, DUPLEX_HALF,
>> and DUPLEX_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
>> ---
>>  include/net/eth.h | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/include/net/eth.h b/include/net/eth.h
>> index 09054a5..9843678 100644
>> --- a/include/net/eth.h
>> +++ b/include/net/eth.h
>> @@ -417,4 +417,11 @@ bool
>>  eth_parse_ipv6_hdr(const struct iovec *pkt, int pkt_frags,
>>                     size_t ip6hdr_off, eth_ip6_hdr_info *info);
>>  
>> +/* ethtool defines - from linux/ethtool.h */
>> +#define SPEED_UNKNOWN           -1
>> +
>> +#define DUPLEX_HALF             0x00
>> +#define DUPLEX_FULL             0x01
>> +#define DUPLEX_UNKNOWN          0xff
>> +
>>  #endif
> 
> While that's not a lot, I think we should import linux/ethtool.h into
> include/standard-headers/linux/ using scripts/update-linux-headers.sh
> 

Ok, I had started down that path, by including
include/uapi/linux/ethtool.h but that resulted in a few other headers -
kernel.h, sysinfo.h. And so it seemed like a lot of headers for only a
few lines. But I will re-visit it...

Thanks,

-Jason

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] Re: [PATCH 4/4] virtio-net: add linkspeed and duplex settings to virtio-net

[Jason Baron]

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

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

Re: [virtio-dev] [PATCH 4/4] virtio-net: add linkspeed and duplex settings to virtio-net

[Jason Baron]

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

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

Re: [virtio-dev] [PATCH 4/4] virtio-net: add linkspeed and duplex settings to virtio-net

[Michael S. Tsirkin]

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.

-- 
MST

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] Re: [PATCH 1/4] eth: add speed and duplex definitions

[Michael S. Tsirkin]

On Tue, Mar 06, 2018 at 12:53:14PM -0500, Jason Baron wrote:
> 
> 
> On 03/02/2018 12:54 PM, Michael S. Tsirkin wrote:
> > On Thu, Mar 01, 2018 at 10:46:33PM -0500, Jason Baron wrote:
> >> Pull in definitions for SPEED_UNKNOWN, DUPLEX_UNKNOWN, DUPLEX_HALF,
> >> and DUPLEX_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
> >> ---
> >>  include/net/eth.h | 7 +++++++
> >>  1 file changed, 7 insertions(+)
> >>
> >> diff --git a/include/net/eth.h b/include/net/eth.h
> >> index 09054a5..9843678 100644
> >> --- a/include/net/eth.h
> >> +++ b/include/net/eth.h
> >> @@ -417,4 +417,11 @@ bool
> >>  eth_parse_ipv6_hdr(const struct iovec *pkt, int pkt_frags,
> >>                     size_t ip6hdr_off, eth_ip6_hdr_info *info);
> >>  
> >> +/* ethtool defines - from linux/ethtool.h */
> >> +#define SPEED_UNKNOWN           -1
> >> +
> >> +#define DUPLEX_HALF             0x00
> >> +#define DUPLEX_FULL             0x01
> >> +#define DUPLEX_UNKNOWN          0xff
> >> +
> >>  #endif
> > 
> > While that's not a lot, I think we should import linux/ethtool.h into
> > include/standard-headers/linux/ using scripts/update-linux-headers.sh
> > 
> 
> Ok, I had started down that path, by including
> include/uapi/linux/ethtool.h but that resulted in a few other headers -
> kernel.h, sysinfo.h. And so it seemed like a lot of headers for only a
> few lines. But I will re-visit it...
> 
> Thanks,
> 
> -Jason

I don't know why is sysinfo there. Want to try sending a patch to
drop it from linux/kernel.h?

-- 
MST

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] Re: [PATCH 1/4] eth: add speed and duplex definitions

[Jason Baron]

On 03/06/2018 01:15 PM, Michael S. Tsirkin wrote:
> On Tue, Mar 06, 2018 at 12:53:14PM -0500, Jason Baron wrote:
>>
>>
>> On 03/02/2018 12:54 PM, Michael S. Tsirkin wrote:
>>> On Thu, Mar 01, 2018 at 10:46:33PM -0500, Jason Baron wrote:
>>>> Pull in definitions for SPEED_UNKNOWN, DUPLEX_UNKNOWN, DUPLEX_HALF,
>>>> and DUPLEX_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
>>>> ---
>>>>  include/net/eth.h | 7 +++++++
>>>>  1 file changed, 7 insertions(+)
>>>>
>>>> diff --git a/include/net/eth.h b/include/net/eth.h
>>>> index 09054a5..9843678 100644
>>>> --- a/include/net/eth.h
>>>> +++ b/include/net/eth.h
>>>> @@ -417,4 +417,11 @@ bool
>>>>  eth_parse_ipv6_hdr(const struct iovec *pkt, int pkt_frags,
>>>>                     size_t ip6hdr_off, eth_ip6_hdr_info *info);
>>>>  
>>>> +/* ethtool defines - from linux/ethtool.h */
>>>> +#define SPEED_UNKNOWN           -1
>>>> +
>>>> +#define DUPLEX_HALF             0x00
>>>> +#define DUPLEX_FULL             0x01
>>>> +#define DUPLEX_UNKNOWN          0xff
>>>> +
>>>>  #endif
>>>
>>> While that's not a lot, I think we should import linux/ethtool.h into
>>> include/standard-headers/linux/ using scripts/update-linux-headers.sh
>>>
>>
>> Ok, I had started down that path, by including
>> include/uapi/linux/ethtool.h but that resulted in a few other headers -
>> kernel.h, sysinfo.h. And so it seemed like a lot of headers for only a
>> few lines. But I will re-visit it...
>>
>> Thanks,
>>
>> -Jason
> 
> I don't know why is sysinfo there. Want to try sending a patch to
> drop it from linux/kernel.h?
> 

Seems like this also ripples into glibc headers, if you look at:
/usr/include/x86_64-linux-gnu/sys/sysinfo.h. It also includes kernel.h
in order to get struct sysinfo. So that would need updating as well.

I've done a v2 that just pulls in sysinfo.h, it doesn't look too bad,
but let me know...

Thanks,

-Jason


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

Re: [virtio-dev] [PATCH 4/4] virtio-net: add linkspeed and duplex settings to virtio-net

[Yan Vugenfirer]

> 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


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org