Re: [PATCH 6/6] crypto: stm32-cryp: convert to the new crypto engine API

[Fabien DESSENNE]

(Adding my tested by)


On 10/01/18 15:25, Fabien DESSENNE wrote:
>
> On 03/01/18 21:11, Corentin Labbe wrote:
>> This patch convert the stm32-cryp driver to the new crypto engine API.
>> Signed-off-by: Corentin Labbe 

Tested-by: Fabien Dessenne 

>> ---
>>drivers/crypto/stm32/stm32-cryp.c | 21 -
>>1 file changed, 16 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/crypto/stm32/stm32-cryp.c 
>> b/drivers/crypto/stm32/stm32-cryp.c
>> index cf1dddbeaa2c..99e0473ef247 100644
>> --- a/drivers/crypto/stm32/stm32-cryp.c
>> +++ b/drivers/crypto/stm32/stm32-cryp.c
>> @@ -91,6 +91,7 @@
>>#define _walked_out (cryp->out_walk.offset - 
>> cryp->out_sg->offset)
>>
>>struct stm32_cryp_ctx {
>> +struct crypto_engine_reqctx enginectx;
>>  struct stm32_cryp   *cryp;
>>  int keylen;
>>  u32 key[AES_KEYSIZE_256 / sizeof(u32)];
>> @@ -494,10 +495,20 @@ static int stm32_cryp_cpu_start(struct stm32_cryp 
>> *cryp)
>>  return 0;
>>}
>>
>> +static int stm32_cryp_cipher_one_req(struct crypto_engine *engine,
>> + void *areq);
> Merge these 2 lines in a single one
>
>> +static int stm32_cryp_prepare_cipher_req(struct crypto_engine *engine,
>> + void *areq);
>> +
>>static int stm32_cryp_cra_init(struct crypto_tfm *tfm)
>>{
>> +struct stm32_cryp_ctx *ctx = crypto_tfm_ctx(tfm);
>> +
>>  tfm->crt_ablkcipher.reqsize = sizeof(struct stm32_cryp_reqctx);
>>
>> +ctx->enginectx.op.do_one_request = stm32_cryp_cipher_one_req;
>> +ctx->enginectx.op.prepare_request = stm32_cryp_prepare_cipher_req;
>> +ctx->enginectx.op.unprepare_request = NULL;
>>  return 0;
>>}
>>
>> @@ -695,14 +706,17 @@ static int stm32_cryp_prepare_req(struct crypto_engine 
>> *engine,
>>}
>>
>>static int stm32_cryp_prepare_cipher_req(struct crypto_engine *engine,
>> - struct ablkcipher_request *req)
>> + void *areq)
>>{
>> +struct ablkcipher_request *req = container_of(areq, struct 
>> ablkcipher_request, base);
>   > 80 characters (CHECKPATCH)
>
>> +
>>  return stm32_cryp_prepare_req(engine, req);
>>}
>>
>>static int stm32_cryp_cipher_one_req(struct crypto_engine *engine,
>> - struct ablkcipher_request *req)
>> + void *areq)
> Merge these 2 lines in a single one
>
>>{
>> +struct ablkcipher_request *req = container_of(areq, struct 
>> ablkcipher_request, base);
>   > 80 characters (CHECKPATCH)
>
>>  struct stm32_cryp_ctx *ctx = crypto_ablkcipher_ctx(
>>  crypto_ablkcipher_reqtfm(req));
>>  struct stm32_cryp *cryp = ctx->cryp;
>> @@ -1104,9 +1118,6 @@ static int stm32_cryp_probe(struct platform_device 
>> *pdev)
>>  goto err_engine1;
>>  }
>>
>> -cryp->engine->prepare_cipher_request = stm32_cryp_prepare_cipher_req;
>> -cryp->engine->cipher_one_request = stm32_cryp_cipher_one_req;
>> -
>>  ret = crypto_engine_start(cryp->engine);
>>  if (ret) {
>>  dev_err(dev, "Could not start crypto engine\n");
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 5/6] crypto: stm32-hash: convert to the new crypto engine API

[Fabien DESSENNE]

(adding my tested my)


On 10/01/18 15:24, Fabien DESSENNE wrote:
>
> On 03/01/18 21:11, Corentin Labbe wrote:
>> This patch convert the stm32-hash driver to the new crypto engine API.
>>
>> Signed-off-by: Corentin Labbe 

Tested-by: Fabien Dessenne 

>> ---
>>drivers/crypto/stm32/stm32-hash.c | 18 +-
>>1 file changed, 13 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/crypto/stm32/stm32-hash.c 
>> b/drivers/crypto/stm32/stm32-hash.c
>> index 4ca4a264a833..9790c2c936c7 100644
>> --- a/drivers/crypto/stm32/stm32-hash.c
>> +++ b/drivers/crypto/stm32/stm32-hash.c
>> @@ -122,6 +122,7 @@ enum stm32_hash_data_format {
>>#define HASH_DMA_THRESHOLD50
>>
>>struct stm32_hash_ctx {
>> +struct crypto_engine_reqctx enginectx;
>>  struct stm32_hash_dev   *hdev;
>>  unsigned long   flags;
>>
>> @@ -828,6 +829,11 @@ static int stm32_hash_hw_init(struct stm32_hash_dev 
>> *hdev,
>>  return 0;
>>}
>>
>> +static int stm32_hash_one_request(struct crypto_engine *engine,
>> +  void *areq);
> merge these two lines in a single one
>
>> +static int stm32_hash_prepare_req(struct crypto_engine *engine,
>> +  void *areq);
> merge these two lines in a single one
>
>> +
>>static int stm32_hash_handle_queue(struct stm32_hash_dev *hdev,
>> struct ahash_request *req)
>>{
>> @@ -835,8 +841,9 @@ static int stm32_hash_handle_queue(struct stm32_hash_dev 
>> *hdev,
>>}
>>
>>static int stm32_hash_prepare_req(struct crypto_engine *engine,
>> -  struct ahash_request *req)
>> +  void *areq)
> merge these two lines in a single one
>
>>{
>> +struct ahash_request *req = container_of(areq, struct ahash_request, 
>> base);
>   > 80 characters (CHECKPATCH)
>
>>  struct stm32_hash_ctx *ctx = crypto_ahash_ctx(crypto_ahash_reqtfm(req));
>>  struct stm32_hash_dev *hdev = stm32_hash_find_dev(ctx);
>>  struct stm32_hash_request_ctx *rctx;
>> @@ -855,8 +862,9 @@ static int stm32_hash_prepare_req(struct crypto_engine 
>> *engine,
>>}
>>
>>static int stm32_hash_one_request(struct crypto_engine *engine,
>> -  struct ahash_request *req)
>> +  void *areq)
> merge these two lines in a single one
>
>>{
>> +struct ahash_request *req = container_of(areq, struct ahash_request, 
>> base);
>   > 80 characters (CHECKPATCH)
>
>>  struct stm32_hash_ctx *ctx = crypto_ahash_ctx(crypto_ahash_reqtfm(req));
>>  struct stm32_hash_dev *hdev = stm32_hash_find_dev(ctx);
>>  struct stm32_hash_request_ctx *rctx;
>> @@ -1033,6 +1041,9 @@ static int stm32_hash_cra_init_algs(struct crypto_tfm 
>> *tfm,
>>  if (algs_hmac_name)
>>  ctx->flags |= HASH_FLAGS_HMAC;
>>
>> +ctx->enginectx.op.do_one_request = stm32_hash_one_request;
>> +ctx->enginectx.op.prepare_request = stm32_hash_prepare_req;
>> +ctx->enginectx.op.unprepare_request = NULL;
>>  return 0;
>>}
>>
>> @@ -1493,9 +1504,6 @@ static int stm32_hash_probe(struct platform_device 
>> *pdev)
>>  goto err_engine;
>>  }
>>
>> -hdev->engine->prepare_hash_request = stm32_hash_prepare_req;
>> -hdev->engine->hash_one_request = stm32_hash_one_request;
>> -
>>  ret = crypto_engine_start(hdev->engine);
>>  if (ret)
>>  goto err_engine_start;
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 2/6] crypto: engine - Permit to enqueue all async requests

[Fabien DESSENNE]

(adding my tested by)


On 10/01/18 15:19, Fabien DESSENNE wrote:
> On 03/01/18 21:11, Corentin Labbe wrote:
>> The crypto engine could actually only enqueue hash and ablkcipher request.
>> This patch permit it to enqueue any type of crypto_async_request.
>>
>> Signed-off-by: Corentin Labbe 

Tested-by: Fabien Dessenne 

>> ---
>>crypto/crypto_engine.c  | 230 
>> 
>>include/crypto/engine.h |  59 +++--
>>2 files changed, 148 insertions(+), 141 deletions(-)
>>
>> diff --git a/crypto/crypto_engine.c b/crypto/crypto_engine.c
>> index 61e7c4e02fd2..036270b61648 100644
>> --- a/crypto/crypto_engine.c
>> +++ b/crypto/crypto_engine.c
>> @@ -15,7 +15,6 @@
>>#include 
>>#include 
>>#include 
>> -#include 
>>#include 
>>#include "internal.h"
>>
>> @@ -34,11 +33,10 @@ static void crypto_pump_requests(struct crypto_engine 
>> *engine,
>>   bool in_kthread)
>>{
>>  struct crypto_async_request *async_req, *backlog;
>> -struct ahash_request *hreq;
>> -struct ablkcipher_request *breq;
>>  unsigned long flags;
>>  bool was_busy = false;
>> -int ret, rtype;
>> +int ret;
>> +struct crypto_engine_reqctx *enginectx;
>>
>>  spin_lock_irqsave(>queue_lock, flags);
>>
>> @@ -94,7 +92,6 @@ static void crypto_pump_requests(struct crypto_engine 
>> *engine,
>>
>>  spin_unlock_irqrestore(>queue_lock, flags);
>>
>> -rtype = crypto_tfm_alg_type(engine->cur_req->tfm);
>>  /* Until here we get the request need to be encrypted successfully */
>>  if (!was_busy && engine->prepare_crypt_hardware) {
>>  ret = engine->prepare_crypt_hardware(engine);
>> @@ -104,57 +101,31 @@ static void crypto_pump_requests(struct crypto_engine 
>> *engine,
>>  }
>>  }
>>
>> -switch (rtype) {
>> -case CRYPTO_ALG_TYPE_AHASH:
>> -hreq = ahash_request_cast(engine->cur_req);
>> -if (engine->prepare_hash_request) {
>> -ret = engine->prepare_hash_request(engine, hreq);
>> -if (ret) {
>> -dev_err(engine->dev, "failed to prepare 
>> request: %d\n",
>> -ret);
>> -goto req_err;
>> -}
>> -engine->cur_req_prepared = true;
>> -}
>> -ret = engine->hash_one_request(engine, hreq);
>> -if (ret) {
>> -dev_err(engine->dev, "failed to hash one request from 
>> queue\n");
>> -goto req_err;
>> -}
>> -return;
>> -case CRYPTO_ALG_TYPE_ABLKCIPHER:
>> -breq = ablkcipher_request_cast(engine->cur_req);
>> -if (engine->prepare_cipher_request) {
>> -ret = engine->prepare_cipher_request(engine, breq);
>> -if (ret) {
>> -dev_err(engine->dev, "failed to prepare 
>> request: %d\n",
>> -ret);
>> -goto req_err;
>> -}
>> -engine->cur_req_prepared = true;
>> -}
>> -ret = engine->cipher_one_request(engine, breq);
>> +enginectx = crypto_tfm_ctx(async_req->tfm);
>> +
>> +if (enginectx->op.prepare_request) {
>> +ret = enginectx->op.prepare_request(engine, async_req);
>>  if (ret) {
>> -dev_err(engine->dev, "failed to cipher one request from 
>> queue\n");
>> +dev_err(engine->dev, "failed to prepare request: %d\n",
>> +ret);
>>  goto req_err;
>>  }
>> -return;
>> -default:
>> -dev_err(engine->dev, "failed to prepare request of unknown 
>> type\n");
>> -return;
>> +engine->cur_req_prepared = true;
>> +}
>> +if (!enginectx->op.do_one_request) {
>> +dev_err(engine->dev, "failed to do request\n");
>> +ret = -EINVAL;
>> +goto req_err;
>> +}
>> +ret = enginectx->op.do_one_request(engine, async_req);
>> +if (ret) {
>> +dev_err(engine->dev, "Failed to do one request from queue: 
>> %d\n", ret);
>> +goto req_err;
>>  }
>> +return;
>>
>>req_err:
>> -switch (rtype) {
>> -case CRYPTO_ALG_TYPE_AHASH:
>> -hreq = ahash_request_cast(engine->cur_req);
>> -crypto_finalize_hash_request(engine, hreq, ret);
>> -break;
>> -case CRYPTO_ALG_TYPE_ABLKCIPHER:
>> -breq = ablkcipher_request_cast(engine->cur_req);
>> -crypto_finalize_cipher_request(engine, breq, ret);
>> -break;
>> -}
>> +crypto_finalize_request(engine, async_req, ret);
>>  return;
>>
>>out:
>> @@ 

Re: [PATCH net-next] vhost_net: batch used ring update in rx

[David Miller]

From: Jason Wang 
Date: Tue,  9 Jan 2018 18:27:45 +0800

> This patch tries to batched used ring update during RX. This is pretty
> fit for the case when guest is much faster (e.g dpdk based
> backend). In this case, used ring is almost empty:
> 
> - we may get serious cache line misses/contending on both used ring
>   and used idx.
> - at most 1 packet could be dequeued at one time, batching in guest
>   does not make much effect.
> 
> Update used ring in a batch can help since guest won't access the used
> ring until used idx was advanced for several descriptors and since we
> advance used ring for every N packets, guest will only need to access
> used idx for every N packet since it can cache the used idx. To have a
> better interaction for both batch dequeuing and dpdk batching,
> VHOST_RX_BATCH was used as the maximum number of descriptors that
> could be batched.
> 
> Test were done between two machines with 2.40GHz Intel(R) Xeon(R) CPU
> E5-2630 connected back to back through ixgbe. Traffic were generated
> on one remote ixgbe through MoonGen and measure the RX pps through
> testpmd in guest when do xdp_redirect_map from local ixgbe to
> tap. RX pps were increased from 3.05 Mpps to 4.00 Mpps (about 31%
> improvement).
> 
> One possible concern for this is the implications for TCP (especially
> latency sensitive workload). Result[1] does not show obvious changes
> for most of the netperf test (RR, TX, and RX). And we do get some
> improvements for RX on some specific size.
 ...
> Signed-off-by: Jason Wang 

Applied, thanks Jason.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: Question about eliminating virtio-scsi 30s timeout and hot-unplug

[Hannes Reinecke]

On 12/21/2017 08:29 PM, Jason Behmer via Virtualization wrote:
> Hi Paolo,
> I had a question about this patch that eliminates virtio-scsi timeout on
> IOs - https://patchwork.kernel.org/patch/9802047/.  How does this work
> when we have IOs outstanding to a device that is then hot-unplugged. 
> I'm under the impression that those IOs will never get returned with any
> status (please correct me if I'm wrong), and we will then end up waiting
> on them forever with this patch.
> 
It's no different from the virtio-block path; the I/O is pending in the
qemu process. When a device is hot-unplugged the I/O will be returned to
the qemu process, which then will return the final status to the guest.

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 1/6] Documentation: crypto: document crypto engine API

[Corentin Labbe]

On Wed, Jan 10, 2018 at 02:13:13PM +, Fabien DESSENNE wrote:
> Hi Corentin,
> 
> 
> Thank you for this new version which I have testes successfully with the 
> stm32 hash & cryp drivers.
> 
> As a general comment on this patchset, I would say that it does not 
> cover all async requests: typically I need (for the pending stm32 cryp 
> driver uprade) to use CryptoEngine to process AEAD requests which is not 
> covered here.
> 
> Could you please consider adding the 'transfer' and 'finalize' EXPORTed 
> functions for aead requests? (the implementation is quite trivial)
> 
> Have also a look at struct acomp_req (acompress.h) and struct 
> kpp_request (kpp.h) which also use "struct crypto_async_request base"
> 
> 
> BR
> 
> Fabien
> 

Hello

Thanks for your review and test (could I add your tested-by ?).
I didn't add aead (and kpp/acompress), since I do not have any way to test it.
Since you have a way to test aead, I will add it to the next release.

Regards

> 
> On 03/01/18 21:11, Corentin Labbe wrote:
> > Signed-off-by: Corentin Labbe 
> > ---
> >   Documentation/crypto/crypto_engine.rst | 46 
> > ++
> >   1 file changed, 46 insertions(+)
> >   create mode 100644 Documentation/crypto/crypto_engine.rst
> >
> > diff --git a/Documentation/crypto/crypto_engine.rst 
> > b/Documentation/crypto/crypto_engine.rst
> > new file mode 100644
> > index ..b0ed37f9fb0c
> > --- /dev/null
> > +++ b/Documentation/crypto/crypto_engine.rst
> > @@ -0,0 +1,46 @@
> > +=
> > +CRYPTO ENGINE
> > +=
> > +
> > +Overview
> > +
> > +The crypto engine API (CE), is a crypto queue manager.
> > +
> > +Requirement
> > +---
> > +You have to put at start of your tfm_ctx the struct crypto_engine_reqctx
> > +struct your_tfm_ctx {
> > +struct crypto_engine_reqctx enginectx;
> > +...
> > +};
> > +Why: Since CE manage only crypto_async_request, it cannot know the 
> > underlying
> > +request_type and so have access only on the TFM.
> > +So using container_of for accessing __ctx is impossible.
> > +Furthermore, the crypto engine cannot know the "struct your_tfm_ctx",
> > +so it must assume that crypto_engine_reqctx is at start of it.
> > +
> > +Order of operations
> > +---
> > +You have to obtain a struct crypto_engine via crypto_engine_alloc_init().
> > +And start it via crypto_engine_start().
> > +
> > +Before transferring any request, you have to fill the enginectx.
> > +- prepare_request: (taking a function pointer) If you need to do some 
> > processing before doing the request
> > +- unprepare_request: (taking a function pointer) Undoing what's done in 
> > prepare_request
> > +- do_one_request: (taking a function pointer) Do encryption for current 
> > request
> > +
> > +Note: that those three functions get the crypto_async_request associated 
> > with the received request.
> > +So your need to get the original request via container_of(areq, struct 
> > yourrequesttype_request, base);
> > +
> > +When your driver receive a crypto_request, you have to transfer it to
> > +the cryptoengine via one of:
> > +- crypto_transfer_cipher_request_to_engine()
> > +- crypto_transfer_skcipher_request_to_engine()
> > +- crypto_transfer_akcipher_request_to_engine()
> > +- crypto_transfer_hash_request_to_engine()
> > +
> > +At the end of the request process, a call to one of the following function 
> > is needed:
> > +- crypto_finalize_cipher_request
> > +- crypto_finalize_skcipher_request
> > +- crypto_finalize_akcipher_request
> > +- crypto_finalize_hash_request
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH v4 2/3] qemu: virtio-net: use 64-bit values for feature flags

[Jason Baron via Virtualization]

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 
Cc: "Michael S. Tsirkin" 
Cc: Jason Wang 
Cc: virtio-...@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 38674b0..54823af 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),
-

[PATCH net-next v4 1/3] virtio_net: propagate linkspeed/duplex settings from the hypervisor

[Jason Baron via Virtualization]

The ability to set speed and duplex for virtio_net is useful in various
scenarios as described here:

16032be virtio_net: add ethtool support for set and get of settings

However, it would be nice to be able to set this from the hypervisor,
such that virtio_net doesn't require custom guest ethtool commands.

Introduce a new feature flag, VIRTIO_NET_F_SPEED_DUPLEX, which allows
the hypervisor to export a linkspeed and duplex setting. The user can
subsequently overwrite it later if desired via: 'ethtool -s'.

Note that VIRTIO_NET_F_SPEED_DUPLEX is defined as bit 63, the intention
is that device feature bits are to grow down from bit 63, since the
transports are starting from bit 24 and growing up.

Signed-off-by: Jason Baron 
Cc: "Michael S. Tsirkin" 
Cc: Jason Wang 
Cc: virtio-...@lists.oasis-open.org
---
 drivers/net/virtio_net.c| 23 ++-
 include/uapi/linux/virtio_net.h | 13 +
 2 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 6fb7b65..4f27508 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1894,6 +1894,24 @@ static void virtnet_init_settings(struct net_device *dev)
vi->duplex = DUPLEX_UNKNOWN;
 }
 
+static void virtnet_update_settings(struct virtnet_info *vi)
+{
+   u32 speed;
+   u8 duplex;
+
+   if (!virtio_has_feature(vi->vdev, VIRTIO_NET_F_SPEED_DUPLEX))
+   return;
+
+   speed = virtio_cread32(vi->vdev, offsetof(struct virtio_net_config,
+ speed));
+   if (ethtool_validate_speed(speed))
+   vi->speed = speed;
+   duplex = virtio_cread8(vi->vdev, offsetof(struct virtio_net_config,
+ duplex));
+   if (ethtool_validate_duplex(duplex))
+   vi->duplex = duplex;
+}
+
 static const struct ethtool_ops virtnet_ethtool_ops = {
.get_drvinfo = virtnet_get_drvinfo,
.get_link = ethtool_op_get_link,
@@ -2147,6 +2165,7 @@ static void virtnet_config_changed_work(struct 
work_struct *work)
vi->status = v;
 
if (vi->status & VIRTIO_NET_S_LINK_UP) {
+   virtnet_update_settings(vi);
netif_carrier_on(vi->dev);
netif_tx_wake_all_queues(vi->dev);
} else {
@@ -2695,6 +2714,7 @@ static int virtnet_probe(struct virtio_device *vdev)
schedule_work(>config_work);
} else {
vi->status = VIRTIO_NET_S_LINK_UP;
+   virtnet_update_settings(vi);
netif_carrier_on(dev);
}
 
@@ -2796,7 +2816,8 @@ static struct virtio_device_id id_table[] = {
VIRTIO_NET_F_CTRL_RX, VIRTIO_NET_F_CTRL_VLAN, \
VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ, \
VIRTIO_NET_F_CTRL_MAC_ADDR, \
-   VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS
+   VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, \
+   VIRTIO_NET_F_SPEED_DUPLEX
 
 static unsigned int features[] = {
VIRTNET_FEATURES,
diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
index fc353b5..5de6ed3 100644
--- a/include/uapi/linux/virtio_net.h
+++ b/include/uapi/linux/virtio_net.h
@@ -57,6 +57,8 @@
 * Steering */
 #define VIRTIO_NET_F_CTRL_MAC_ADDR 23  /* Set MAC address */
 
+#define VIRTIO_NET_F_SPEED_DUPLEX 63   /* Device set linkspeed and duplex */
+
 #ifndef VIRTIO_NET_NO_LEGACY
 #define VIRTIO_NET_F_GSO   6   /* Host handles pkts w/ any GSO type */
 #endif /* VIRTIO_NET_NO_LEGACY */
@@ -76,6 +78,17 @@ struct virtio_net_config {
__u16 max_virtqueue_pairs;
/* Default maximum transmit unit advice */
__u16 mtu;
+   /*
+* speed, in units of 1Mb. All values 0 to INT_MAX are legal.
+* Any other value stands for unknown.
+*/
+   __u32 speed;
+   /*
+* 0x00 - half duplex
+* 0x01 - full duplex
+* Any other value stands for unknown.
+*/
+   __u8 duplex;
 } __attribute__((packed));
 
 /*
-- 
2.6.1

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH v4 0/3] virtio_net: allow hypervisor to indicate linkspeed and duplex setting

[Jason Baron via Virtualization]

We have found it useful to be able to set the linkspeed and duplex
settings from the host-side for virtio_net. This obviates the need
for guest changes and settings for these fields, and does not require
custom ethtool commands for virtio_net.

The ability to set linkspeed and duplex is useful in various cases
as described here:

16032be virtio_net: add ethtool support for set and get of settings

Using 'ethtool -s' continues to over-write the linkspeed/duplex
settings with this patch.

The 1/3 patch is against net-next, while the 2-3/3 patch are the associated
qemu changes that would go in after as update-linux-headers.sh should
be run first. So the qemu patches are a demonstration of how I intend this
to work.

Thanks,

-Jason

linux changes:

changes from v3:
* break the speed/duplex read into a function and also call from virtnet_probe
  when status bit is not negotiated
* only do speed/duplex read in virtnet_config_changed_work() on LINK_UP

changes from v2:
* move speed/duplex read into virtnet_config_changed_work() so link up changes
  are detected

Jason Baron (1):
  virtio_net: propagate linkspeed/duplex settings from the hypervisor

 drivers/net/virtio_net.c| 23 ++-
 include/uapi/linux/virtio_net.h | 13 +
 2 files changed, 35 insertions(+), 1 deletion(-)


qemu changes:

Jason Baron (2):
  qemu: virtio-net: use 64-bit values for feature flags
  qemu: add linkspeed and duplex settings to virtio-net

 hw/net/virtio-net.c | 87 -
 include/hw/virtio/virtio-net.h  |  5 +-
 include/standard-headers/linux/virtio_net.h | 13 +
 3 files changed, 77 insertions(+), 28 deletions(-)

-- 
2.6.1

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH v2] vhost: remove unused lock check flag in vhost_dev_cleanup()

[夷则(Caspar)]

From: Caspar Zhang 

In commit ea5d404655ba ("vhost: fix release path lockdep checks"),
Michael added a flag to check whether we should hold a lock in
vhost_dev_cleanup(), however, in commit 47283bef7ed3 ("vhost: move
memory pointer to VQs"), RCU operations have been replaced by
mutex, we can remove the no-longer-used `locked' parameter now.

Signed-off-by: Caspar Zhang 
Acked-by: Jason Wang 
---
v1->v2: fix author name with UTF-8 characters.

 drivers/vhost/net.c   | 2 +-
 drivers/vhost/scsi.c  | 2 +-
 drivers/vhost/test.c  | 2 +-
 drivers/vhost/vhost.c | 5 ++---
 drivers/vhost/vhost.h | 2 +-
 drivers/vhost/vsock.c | 2 +-
 6 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index c7bdeb655646..a354d8d731e3 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -996,7 +996,7 @@ static int vhost_net_release(struct inode *inode, struct 
file *f)
vhost_net_stop(n, _sock, _sock);
vhost_net_flush(n);
vhost_dev_stop(>dev);
-   vhost_dev_cleanup(>dev, false);
+   vhost_dev_cleanup(>dev);
vhost_net_vq_reset(n);
if (tx_sock)
sockfd_put(tx_sock);
diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index 71517b3c5558..797d08916553 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -1420,7 +1420,7 @@ static int vhost_scsi_release(struct inode *inode, struct 
file *f)
mutex_unlock(>dev.mutex);
vhost_scsi_clear_endpoint(vs, );
vhost_dev_stop(>dev);
-   vhost_dev_cleanup(>dev, false);
+   vhost_dev_cleanup(>dev);
/* Jobs can re-queue themselves in evt kick handler. Do extra flush. */
vhost_scsi_flush(vs);
kfree(vs->dev.vqs);
diff --git a/drivers/vhost/test.c b/drivers/vhost/test.c
index 3cc98c07dcd3..906b8f0f19f7 100644
--- a/drivers/vhost/test.c
+++ b/drivers/vhost/test.c
@@ -157,7 +157,7 @@ static int vhost_test_release(struct inode *inode, struct 
file *f)
 
vhost_test_stop(n, );
vhost_test_flush(n);
-   vhost_dev_cleanup(>dev, false);
+   vhost_dev_cleanup(>dev);
/* We do an extra flush before freeing memory,
 * since jobs can re-queue themselves. */
vhost_test_flush(n);
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 33ac2b186b85..014675c3d569 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -544,7 +544,7 @@ void vhost_dev_reset_owner(struct vhost_dev *dev, struct 
vhost_umem *umem)
 {
int i;
 
-   vhost_dev_cleanup(dev, true);
+   vhost_dev_cleanup(dev);
 
/* Restore memory to default empty mapping. */
INIT_LIST_HEAD(>umem_list);
@@ -611,8 +611,7 @@ static void vhost_clear_msg(struct vhost_dev *dev)
spin_unlock(>iotlb_lock);
 }
 
-/* Caller should have device mutex if and only if locked is set */
-void vhost_dev_cleanup(struct vhost_dev *dev, bool locked)
+void vhost_dev_cleanup(struct vhost_dev *dev)
 {
int i;
 
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index 79c6e7a60a5e..ff4d918e3e0a 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -181,7 +181,7 @@ bool vhost_dev_has_owner(struct vhost_dev *dev);
 long vhost_dev_check_owner(struct vhost_dev *);
 struct vhost_umem *vhost_dev_reset_owner_prepare(void);
 void vhost_dev_reset_owner(struct vhost_dev *, struct vhost_umem *);
-void vhost_dev_cleanup(struct vhost_dev *, bool locked);
+void vhost_dev_cleanup(struct vhost_dev *);
 void vhost_dev_stop(struct vhost_dev *);
 long vhost_dev_ioctl(struct vhost_dev *, unsigned int ioctl, void __user 
*argp);
 long vhost_vring_ioctl(struct vhost_dev *d, int ioctl, void __user *argp);
diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
index 5a5e981bd8e4..0d14e2ff19f1 100644
--- a/drivers/vhost/vsock.c
+++ b/drivers/vhost/vsock.c
@@ -599,7 +599,7 @@ static int vhost_vsock_dev_release(struct inode *inode, 
struct file *file)
}
spin_unlock_bh(>send_pkt_list_lock);
 
-   vhost_dev_cleanup(>dev, false);
+   vhost_dev_cleanup(>dev);
kfree(vsock->dev.vqs);
vhost_vsock_free(vsock);
return 0;
-- 
2.15.1

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH net-next v3 1/3] virtio_net: propagate linkspeed/duplex settings from the hypervisor

[Jason Baron via Virtualization]

On 01/04/2018 11:27 AM, Michael S. Tsirkin wrote:
> On Thu, Jan 04, 2018 at 12:16:44AM -0500, Jason Baron wrote:
>> The ability to set speed and duplex for virtio_net is useful in various
>> scenarios as described here:
>>
>> 16032be virtio_net: add ethtool support for set and get of settings
>>
>> However, it would be nice to be able to set this from the hypervisor,
>> such that virtio_net doesn't require custom guest ethtool commands.
>>
>> Introduce a new feature flag, VIRTIO_NET_F_SPEED_DUPLEX, which allows
>> the hypervisor to export a linkspeed and duplex setting. The user can
>> subsequently overwrite it later if desired via: 'ethtool -s'.
>>
>> Note that VIRTIO_NET_F_SPEED_DUPLEX is defined as bit 63, the intention
>> is that device feature bits are to grow down from bit 63, since the
>> transports are starting from bit 24 and growing up.
>>
>> Signed-off-by: Jason Baron 
>> Cc: "Michael S. Tsirkin" 
>> Cc: Jason Wang 
>> Cc: virtio-...@lists.oasis-open.org
>> ---
>>  drivers/net/virtio_net.c| 19 ++-
>>  include/uapi/linux/virtio_net.h | 13 +
>>  2 files changed, 31 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>> index 6fb7b65..0b2d314 100644
>> --- a/drivers/net/virtio_net.c
>> +++ b/drivers/net/virtio_net.c
>> @@ -2146,6 +2146,22 @@ static void virtnet_config_changed_work(struct 
>> work_struct *work)
>>  
>>  vi->status = v;
>>  
>> +if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_SPEED_DUPLEX)) {
>> +u32 speed;
>> +u8 duplex;
>> +
>> +speed = virtio_cread32(vi->vdev,
>> +   offsetof(struct virtio_net_config,
>> +speed));
>> +if (ethtool_validate_speed(speed))
>> +vi->speed = speed;
>> +duplex = virtio_cread8(vi->vdev,
>> +   offsetof(struct virtio_net_config,
>> +duplex));
>> +if (ethtool_validate_duplex(duplex))
>> +vi->duplex = duplex;
>> +}
>> +
>>  if (vi->status & VIRTIO_NET_S_LINK_UP) {
>>  netif_carrier_on(vi->dev);
>>  netif_tx_wake_all_queues(vi->dev);
>> @@ -2796,7 +2812,8 @@ static struct virtio_device_id id_table[] = {
>>  VIRTIO_NET_F_CTRL_RX, VIRTIO_NET_F_CTRL_VLAN, \
>>  VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ, \
>>  VIRTIO_NET_F_CTRL_MAC_ADDR, \
>> -VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS
>> +VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, \
>> +VIRTIO_NET_F_SPEED_DUPLEX
>>  
>>  static unsigned int features[] = {
>>  VIRTNET_FEATURES,
> 
> Still missing the update from virtnet_config_changed_work, and I think
> it's important to reflex host changes within guest when the
> feature bit has been acked.
> 

I update vi->speed and vi->duplex in virtnet_config_changed_work(). And
I tested using the 'set_link' on/off from the qemu monitor.
Specifically, an 'off' sets the speed and link to 'unknown', and an 'on'
returns the speed and link to the configured speed and duplex. So they
are being updated dynamically now. What host changes are you referring
to that are not reflected?

Thanks,

-Jason


>> diff --git a/include/uapi/linux/virtio_net.h 
>> b/include/uapi/linux/virtio_net.h
>> index fc353b5..5de6ed3 100644
>> --- a/include/uapi/linux/virtio_net.h
>> +++ b/include/uapi/linux/virtio_net.h
>> @@ -57,6 +57,8 @@
>>   * Steering */
>>  #define VIRTIO_NET_F_CTRL_MAC_ADDR 23   /* Set MAC address */
>>  
>> +#define VIRTIO_NET_F_SPEED_DUPLEX 63/* Device set linkspeed and 
>> duplex */
>> +
>>  #ifndef VIRTIO_NET_NO_LEGACY
>>  #define VIRTIO_NET_F_GSO6   /* Host handles pkts w/ any GSO type */
>>  #endif /* VIRTIO_NET_NO_LEGACY */
>> @@ -76,6 +78,17 @@ struct virtio_net_config {
>>  __u16 max_virtqueue_pairs;
>>  /* Default maximum transmit unit advice */
>>  __u16 mtu;
>> +/*
>> + * speed, in units of 1Mb. All values 0 to INT_MAX are legal.
>> + * Any other value stands for unknown.
>> + */
>> +__u32 speed;
>> +/*
>> + * 0x00 - half duplex
>> + * 0x01 - full duplex
>> + * Any other value stands for unknown.
>> + */
>> +__u8 duplex;
>>  } __attribute__((packed));
>>  
>>  /*
>> -- 
>> 2.6.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH net-next v3 1/3] virtio_net: propagate linkspeed/duplex settings from the hypervisor

[Jason Baron via Virtualization]

On 01/04/2018 01:22 PM, Michael S. Tsirkin wrote:
> On Thu, Jan 04, 2018 at 01:12:30PM -0500, Jason Baron wrote:
>>
>>
>> On 01/04/2018 12:05 PM, Michael S. Tsirkin wrote:
>>> On Thu, Jan 04, 2018 at 12:16:44AM -0500, Jason Baron wrote:
 The ability to set speed and duplex for virtio_net is useful in various
 scenarios as described here:

 16032be virtio_net: add ethtool support for set and get of settings

 However, it would be nice to be able to set this from the hypervisor,
 such that virtio_net doesn't require custom guest ethtool commands.

 Introduce a new feature flag, VIRTIO_NET_F_SPEED_DUPLEX, which allows
 the hypervisor to export a linkspeed and duplex setting. The user can
 subsequently overwrite it later if desired via: 'ethtool -s'.

 Note that VIRTIO_NET_F_SPEED_DUPLEX is defined as bit 63, the intention
 is that device feature bits are to grow down from bit 63, since the
 transports are starting from bit 24 and growing up.

 Signed-off-by: Jason Baron 
 Cc: "Michael S. Tsirkin" 
 Cc: Jason Wang 
 Cc: virtio-...@lists.oasis-open.org
 ---
  drivers/net/virtio_net.c| 19 ++-
  include/uapi/linux/virtio_net.h | 13 +
  2 files changed, 31 insertions(+), 1 deletion(-)

 diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
 index 6fb7b65..0b2d314 100644
 --- a/drivers/net/virtio_net.c
 +++ b/drivers/net/virtio_net.c
 @@ -2146,6 +2146,22 @@ static void virtnet_config_changed_work(struct 
 work_struct *work)
  
vi->status = v;
  
 +  if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_SPEED_DUPLEX)) {
>>>
>>> BTW we can avoid this read for when link goes down.
>>> Not a big deal but still.
>>
>> So you are saying that we can just set vi->speed and vi->duplex to
>> 'unknown' when the link goes down and not check for the presence of
>> VIRTIO_NET_F_SPEED_DUPLEX?
>>
>> If so, that could over-write what the user may have configured in the
>> guest via 'ethtool -s' when the link goes down, so that would be a
>> change in behavior, but perhaps that is ok?
> 
> No - what I am saying is that your patch overwrites the values
> set by user when link goes down.
> 
> I suggest limiting this call to when
> 
> if (vi->status & VIRTIO_NET_S_LINK_UP)
> 
> and then the values are overwritten when link goes up
> which seems closer to what a user might expect.
ok, will update this for v4.

Thanks,

-Jason

> 
>>
>> I think I would prefer to have the link down event still check for
>> VIRTIO_NET_F_SPEED_DUPLEX before changing speed/duplex. That way we
>> still have 2 modes for updating the fields:
>>
>> 1) completely guest controlled. Same as we have now and host does not
>> change any values and does not set VIRTIO_NET_F_SPEED_DUPLEX flag (hence
>> don't remove above check).
>>
>> 2) if speed or duplex or speed is set in the qemu command line, then set
>> the VIRTIO_NET_F_SPEED_DUPLEX and have host control the settings of
>> speed/duplex (with ability of guest to over-write if it wanted to).
>>
>>
>>>
 +  u32 speed;
 +  u8 duplex;
 +
 +  speed = virtio_cread32(vi->vdev,
 + offsetof(struct virtio_net_config,
 +  speed));
 +  if (ethtool_validate_speed(speed))
 +  vi->speed = speed;
 +  duplex = virtio_cread8(vi->vdev,
 + offsetof(struct virtio_net_config,
 +  duplex));
 +  if (ethtool_validate_duplex(duplex))
 +  vi->duplex = duplex;
 +  }
 +
if (vi->status & VIRTIO_NET_S_LINK_UP) {
netif_carrier_on(vi->dev);
netif_tx_wake_all_queues(vi->dev);
>>>
>>> OK so this handles the case when VIRTIO_NET_F_STATUS is set,
>>> but when it's clear we need to call this from virtnet_probe.
>>>
>>> I propose moving this chunk to a function and calling from two places.
>>>
>>
>> good point. will update.
>>
>> Thanks,
>>
>> -Jason
>>
>>>
 @@ -2796,7 +2812,8 @@ static struct virtio_device_id id_table[] = {
VIRTIO_NET_F_CTRL_RX, VIRTIO_NET_F_CTRL_VLAN, \
VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ, \
VIRTIO_NET_F_CTRL_MAC_ADDR, \
 -  VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS
 +  VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, \
 +  VIRTIO_NET_F_SPEED_DUPLEX
  
  static unsigned int features[] = {
VIRTNET_FEATURES,
 diff --git a/include/uapi/linux/virtio_net.h 
 b/include/uapi/linux/virtio_net.h
 index fc353b5..5de6ed3 100644
 --- a/include/uapi/linux/virtio_net.h
 +++ b/include/uapi/linux/virtio_net.h
 @@ -57,6 +57,8 @@
 * Steering */
  #define 

Re: [PATCH net-next v3 1/3] virtio_net: propagate linkspeed/duplex settings from the hypervisor

[Jason Baron via Virtualization]

On 01/04/2018 12:05 PM, Michael S. Tsirkin wrote:
> On Thu, Jan 04, 2018 at 12:16:44AM -0500, Jason Baron wrote:
>> The ability to set speed and duplex for virtio_net is useful in various
>> scenarios as described here:
>>
>> 16032be virtio_net: add ethtool support for set and get of settings
>>
>> However, it would be nice to be able to set this from the hypervisor,
>> such that virtio_net doesn't require custom guest ethtool commands.
>>
>> Introduce a new feature flag, VIRTIO_NET_F_SPEED_DUPLEX, which allows
>> the hypervisor to export a linkspeed and duplex setting. The user can
>> subsequently overwrite it later if desired via: 'ethtool -s'.
>>
>> Note that VIRTIO_NET_F_SPEED_DUPLEX is defined as bit 63, the intention
>> is that device feature bits are to grow down from bit 63, since the
>> transports are starting from bit 24 and growing up.
>>
>> Signed-off-by: Jason Baron 
>> Cc: "Michael S. Tsirkin" 
>> Cc: Jason Wang 
>> Cc: virtio-...@lists.oasis-open.org
>> ---
>>  drivers/net/virtio_net.c| 19 ++-
>>  include/uapi/linux/virtio_net.h | 13 +
>>  2 files changed, 31 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>> index 6fb7b65..0b2d314 100644
>> --- a/drivers/net/virtio_net.c
>> +++ b/drivers/net/virtio_net.c
>> @@ -2146,6 +2146,22 @@ static void virtnet_config_changed_work(struct 
>> work_struct *work)
>>  
>>  vi->status = v;
>>  
>> +if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_SPEED_DUPLEX)) {
> 
> BTW we can avoid this read for when link goes down.
> Not a big deal but still.

So you are saying that we can just set vi->speed and vi->duplex to
'unknown' when the link goes down and not check for the presence of
VIRTIO_NET_F_SPEED_DUPLEX?

If so, that could over-write what the user may have configured in the
guest via 'ethtool -s' when the link goes down, so that would be a
change in behavior, but perhaps that is ok?

I think I would prefer to have the link down event still check for
VIRTIO_NET_F_SPEED_DUPLEX before changing speed/duplex. That way we
still have 2 modes for updating the fields:

1) completely guest controlled. Same as we have now and host does not
change any values and does not set VIRTIO_NET_F_SPEED_DUPLEX flag (hence
don't remove above check).

2) if speed or duplex or speed is set in the qemu command line, then set
the VIRTIO_NET_F_SPEED_DUPLEX and have host control the settings of
speed/duplex (with ability of guest to over-write if it wanted to).


> 
>> +u32 speed;
>> +u8 duplex;
>> +
>> +speed = virtio_cread32(vi->vdev,
>> +   offsetof(struct virtio_net_config,
>> +speed));
>> +if (ethtool_validate_speed(speed))
>> +vi->speed = speed;
>> +duplex = virtio_cread8(vi->vdev,
>> +   offsetof(struct virtio_net_config,
>> +duplex));
>> +if (ethtool_validate_duplex(duplex))
>> +vi->duplex = duplex;
>> +}
>> +
>>  if (vi->status & VIRTIO_NET_S_LINK_UP) {
>>  netif_carrier_on(vi->dev);
>>  netif_tx_wake_all_queues(vi->dev);
> 
> OK so this handles the case when VIRTIO_NET_F_STATUS is set,
> but when it's clear we need to call this from virtnet_probe.
> 
> I propose moving this chunk to a function and calling from two places.
> 

good point. will update.

Thanks,

-Jason

> 
>> @@ -2796,7 +2812,8 @@ static struct virtio_device_id id_table[] = {
>>  VIRTIO_NET_F_CTRL_RX, VIRTIO_NET_F_CTRL_VLAN, \
>>  VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ, \
>>  VIRTIO_NET_F_CTRL_MAC_ADDR, \
>> -VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS
>> +VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, \
>> +VIRTIO_NET_F_SPEED_DUPLEX
>>  
>>  static unsigned int features[] = {
>>  VIRTNET_FEATURES,
>> diff --git a/include/uapi/linux/virtio_net.h 
>> b/include/uapi/linux/virtio_net.h
>> index fc353b5..5de6ed3 100644
>> --- a/include/uapi/linux/virtio_net.h
>> +++ b/include/uapi/linux/virtio_net.h
>> @@ -57,6 +57,8 @@
>>   * Steering */
>>  #define VIRTIO_NET_F_CTRL_MAC_ADDR 23   /* Set MAC address */
>>  
>> +#define VIRTIO_NET_F_SPEED_DUPLEX 63/* Device set linkspeed and 
>> duplex */
>> +
>>  #ifndef VIRTIO_NET_NO_LEGACY
>>  #define VIRTIO_NET_F_GSO6   /* Host handles pkts w/ any GSO type */
>>  #endif /* VIRTIO_NET_NO_LEGACY */
>> @@ -76,6 +78,17 @@ struct virtio_net_config {
>>  __u16 max_virtqueue_pairs;
>>  /* Default maximum transmit unit advice */
>>  __u16 mtu;
>> +/*
>> + * speed, in units of 1Mb. All values 0 to INT_MAX are legal.
>> + * Any other value stands for unknown.
>> + */
>> +__u32 speed;
>> +/*
>> + * 0x00 - 

[PATCH v3 3/3] qemu: add linkspeed and duplex settings to virtio-net

[Jason Baron via Virtualization]

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=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 
Cc: virtio-...@lists.oasis-open.org
---
 hw/net/virtio-net.c | 35 +
 include/hw/virtio/virtio-net.h  |  3 +++
 include/standard-headers/linux/virtio_net.h | 13 +++
 3 files changed, 51 insertions(+)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index adc20df..eec8422 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 */
+#define DUPLEX_UNKNOWN  0xff
+#define DUPLEX_HALF 0x00
+#define DUPLEX_FULL 0x01
+#define SPEED_UNKNOWN   -1
+
 /*
  * Calculate the number of bytes up to and including the given 'field' of
  * 'container'.
@@ -61,6 +67,8 @@ static VirtIOFeature feature_sizes[] = {
  .end = endof(struct virtio_net_config, max_virtqueue_pairs)},
 {.flags = 1ULL << VIRTIO_NET_F_MTU,
  .end = endof(struct virtio_net_config, mtu)},
+{.flags = 1ULL << VIRTIO_NET_F_SPEED_DUPLEX,
+ .end = endof(struct virtio_net_config, duplex)},
 {}
 };
 
@@ -89,6 +97,14 @@ static void virtio_net_get_config(VirtIODevice *vdev, 
uint8_t *config)
 virtio_stw_p(vdev, _virtqueue_pairs, n->max_queues);
 virtio_stw_p(vdev, , n->net_conf.mtu);
 memcpy(netcfg.mac, n->mac, ETH_ALEN);
+if (n->status & VIRTIO_NET_S_LINK_UP) {
+virtio_stl_p(vdev, , n->net_conf.speed);
+netcfg.duplex = n->net_conf.duplex;
+} else {
+virtio_stl_p(vdev, , SPEED_UNKNOWN);
+netcfg.duplex = DUPLEX_UNKNOWN;
+}
+
 memcpy(config, , n->config_size);
 }
 
@@ -1941,6 +1957,23 @@ static void virtio_net_device_realize(DeviceState *dev, 
Error **errp)
 n->host_features |= (1ULL << VIRTIO_NET_F_MTU);
 }
 
+n->host_features |= (1ULL << VIRTIO_NET_F_SPEED_DUPLEX);
+if (n->net_conf.duplex_str) {
+if (strncmp(n->net_conf.duplex_str, "half", 5) == 0) {
+n->net_conf.duplex = DUPLEX_HALF;
+} else if (strncmp(n->net_conf.duplex_str, "full", 5) == 0) {
+n->net_conf.duplex = DUPLEX_FULL;
+} else {
+error_setg(errp, "'duplex' must be 'half' or 'full'");
+}
+} else {
+n->net_conf.duplex = DUPLEX_UNKNOWN;
+}
+if (n->net_conf.speed < SPEED_UNKNOWN) {
+error_setg(errp, "'speed' must be between -1 (SPEED_UNKOWN) and "
+   "INT_MAX");
+}
+
 virtio_net_set_config_size(n, n->host_features);
 virtio_init(vdev, "virtio-net", VIRTIO_ID_NET, n->config_size);
 
@@ -2160,6 +2193,8 @@ static Property virtio_net_properties[] = {
 DEFINE_PROP_UINT16("host_mtu", VirtIONet, net_conf.mtu, 0),
 DEFINE_PROP_BOOL("x-mtu-bypass-backend", VirtIONet, mtu_bypass_backend,
  true),
+DEFINE_PROP_INT32("speed", VirtIONet, net_conf.speed, SPEED_UNKNOWN),
+DEFINE_PROP_STRING("duplex", VirtIONet, net_conf.duplex_str),
 DEFINE_PROP_END_OF_LIST(),
 };
 
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 */
diff --git a/include/standard-headers/linux/virtio_net.h 
b/include/standard-headers/linux/virtio_net.h
index 30ff249..17c8531 100644
--- a/include/standard-headers/linux/virtio_net.h
+++ b/include/standard-headers/linux/virtio_net.h
@@ -57,6 +57,8 @@
 * Steering */
 #define VIRTIO_NET_F_CTRL_MAC_ADDR 23  /* Set MAC address */
 
+#define VIRTIO_NET_F_SPEED_DUPLEX 63   /* Device set linkspeed and duplex */
+
 #ifndef VIRTIO_NET_NO_LEGACY
 #define VIRTIO_NET_F_GSO   6   /* Host handles pkts w/ any GSO type */
 #endif /* VIRTIO_NET_NO_LEGACY */
@@ -76,6 +78,17 @@ struct virtio_net_config {
uint16_t max_virtqueue_pairs;
/* Default maximum transmit unit advice */
uint16_t mtu;
+   /*
+* speed, in units of 1Mb. All values 0 to INT_MAX are 

[PATCH v3 2/3] qemu: virtio-net: use 64-bit values for feature flags

[Jason Baron via Virtualization]

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 
Cc: "Michael S. Tsirkin" 
Cc: Jason Wang 
Cc: virtio-...@lists.oasis-open.org
---
 hw/net/virtio-net.c| 54 +-
 include/hw/virtio/virtio-net.h |  2 +-
 2 files changed, 28 insertions(+), 28 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 38674b0..adc20df 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,45 @@ static const VMStateDescription vmstate_virtio_net = {
 };
 
 static Property virtio_net_properties[] = {
-DEFINE_PROP_BIT("csum", VirtIONet, host_features, VIRTIO_NET_F_CSUM, true),
-DEFINE_PROP_BIT("guest_csum", VirtIONet, host_features,
+DEFINE_PROP_BIT64("csum", VirtIONet, host_features, VIRTIO_NET_F_CSUM, 
true),
+DEFINE_PROP_BIT64("guest_csum", VirtIONet, host_features,
 VIRTIO_NET_F_GUEST_CSUM, true),
-DEFINE_PROP_BIT("gso", VirtIONet, host_features, VIRTIO_NET_F_GSO, true),
-DEFINE_PROP_BIT("guest_tso4", VirtIONet, host_features,
+DEFINE_PROP_BIT64("gso", VirtIONet, host_features, VIRTIO_NET_F_GSO, true),
+DEFINE_PROP_BIT64("guest_tso4", VirtIONet, host_features,
 VIRTIO_NET_F_GUEST_TSO4, true),
-DEFINE_PROP_BIT("guest_tso6", VirtIONet, host_features,
+DEFINE_PROP_BIT64("guest_tso6", VirtIONet, host_features,
 VIRTIO_NET_F_GUEST_TSO6, true),
-DEFINE_PROP_BIT("guest_ecn", VirtIONet, host_features,
+DEFINE_PROP_BIT64("guest_ecn", VirtIONet, host_features,
 VIRTIO_NET_F_GUEST_ECN, true),
-DEFINE_PROP_BIT("guest_ufo", VirtIONet, host_features,
+DEFINE_PROP_BIT64("guest_ufo", VirtIONet, host_features,
 VIRTIO_NET_F_GUEST_UFO, true),
-DEFINE_PROP_BIT("guest_announce", VirtIONet, host_features,
+DEFINE_PROP_BIT64("guest_announce", VirtIONet, host_features,
 VIRTIO_NET_F_GUEST_ANNOUNCE, true),
-DEFINE_PROP_BIT("host_tso4", VirtIONet, host_features,
+DEFINE_PROP_BIT64("host_tso4", VirtIONet, host_features,
 VIRTIO_NET_F_HOST_TSO4, true),
-DEFINE_PROP_BIT("host_tso6", VirtIONet, host_features,
+DEFINE_PROP_BIT64("host_tso6", VirtIONet, host_features,
 VIRTIO_NET_F_HOST_TSO6, true),
-DEFINE_PROP_BIT("host_ecn", VirtIONet, host_features,
+DEFINE_PROP_BIT64("host_ecn", VirtIONet, host_features,
 VIRTIO_NET_F_HOST_ECN, true),
-DEFINE_PROP_BIT("host_ufo", VirtIONet, host_features,
+DEFINE_PROP_BIT64("host_ufo", VirtIONet, host_features,
 VIRTIO_NET_F_HOST_UFO, true),
-DEFINE_PROP_BIT("mrg_rxbuf", VirtIONet, host_features,
+DEFINE_PROP_BIT64("mrg_rxbuf", VirtIONet, host_features,
 VIRTIO_NET_F_MRG_RXBUF, true),
-DEFINE_PROP_BIT("status", VirtIONet, host_features,
+DEFINE_PROP_BIT64("status", VirtIONet, host_features,
 VIRTIO_NET_F_STATUS, true),
-DEFINE_PROP_BIT("ctrl_vq", VirtIONet, host_features,
+DEFINE_PROP_BIT64("ctrl_vq", VirtIONet, host_features,
 VIRTIO_NET_F_CTRL_VQ, true),
-DEFINE_PROP_BIT("ctrl_rx", VirtIONet, host_features,
+DEFINE_PROP_BIT64("ctrl_rx", VirtIONet, host_features,
 VIRTIO_NET_F_CTRL_RX, true),
-DEFINE_PROP_BIT("ctrl_vlan", VirtIONet, host_features,
+DEFINE_PROP_BIT64("ctrl_vlan", VirtIONet, host_features,
 VIRTIO_NET_F_CTRL_VLAN, true),
-DEFINE_PROP_BIT("ctrl_rx_extra", VirtIONet, host_features,
+DEFINE_PROP_BIT64("ctrl_rx_extra", VirtIONet, host_features,
 VIRTIO_NET_F_CTRL_RX_EXTRA, true),
-DEFINE_PROP_BIT("ctrl_mac_addr", 

[PATCH net-next v3 1/3] virtio_net: propagate linkspeed/duplex settings from the hypervisor

[Jason Baron via Virtualization]

The ability to set speed and duplex for virtio_net is useful in various
scenarios as described here:

16032be virtio_net: add ethtool support for set and get of settings

However, it would be nice to be able to set this from the hypervisor,
such that virtio_net doesn't require custom guest ethtool commands.

Introduce a new feature flag, VIRTIO_NET_F_SPEED_DUPLEX, which allows
the hypervisor to export a linkspeed and duplex setting. The user can
subsequently overwrite it later if desired via: 'ethtool -s'.

Note that VIRTIO_NET_F_SPEED_DUPLEX is defined as bit 63, the intention
is that device feature bits are to grow down from bit 63, since the
transports are starting from bit 24 and growing up.

Signed-off-by: Jason Baron 
Cc: "Michael S. Tsirkin" 
Cc: Jason Wang 
Cc: virtio-...@lists.oasis-open.org
---
 drivers/net/virtio_net.c| 19 ++-
 include/uapi/linux/virtio_net.h | 13 +
 2 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 6fb7b65..0b2d314 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -2146,6 +2146,22 @@ static void virtnet_config_changed_work(struct 
work_struct *work)
 
vi->status = v;
 
+   if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_SPEED_DUPLEX)) {
+   u32 speed;
+   u8 duplex;
+
+   speed = virtio_cread32(vi->vdev,
+  offsetof(struct virtio_net_config,
+   speed));
+   if (ethtool_validate_speed(speed))
+   vi->speed = speed;
+   duplex = virtio_cread8(vi->vdev,
+  offsetof(struct virtio_net_config,
+   duplex));
+   if (ethtool_validate_duplex(duplex))
+   vi->duplex = duplex;
+   }
+
if (vi->status & VIRTIO_NET_S_LINK_UP) {
netif_carrier_on(vi->dev);
netif_tx_wake_all_queues(vi->dev);
@@ -2796,7 +2812,8 @@ static struct virtio_device_id id_table[] = {
VIRTIO_NET_F_CTRL_RX, VIRTIO_NET_F_CTRL_VLAN, \
VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ, \
VIRTIO_NET_F_CTRL_MAC_ADDR, \
-   VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS
+   VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, \
+   VIRTIO_NET_F_SPEED_DUPLEX
 
 static unsigned int features[] = {
VIRTNET_FEATURES,
diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
index fc353b5..5de6ed3 100644
--- a/include/uapi/linux/virtio_net.h
+++ b/include/uapi/linux/virtio_net.h
@@ -57,6 +57,8 @@
 * Steering */
 #define VIRTIO_NET_F_CTRL_MAC_ADDR 23  /* Set MAC address */
 
+#define VIRTIO_NET_F_SPEED_DUPLEX 63   /* Device set linkspeed and duplex */
+
 #ifndef VIRTIO_NET_NO_LEGACY
 #define VIRTIO_NET_F_GSO   6   /* Host handles pkts w/ any GSO type */
 #endif /* VIRTIO_NET_NO_LEGACY */
@@ -76,6 +78,17 @@ struct virtio_net_config {
__u16 max_virtqueue_pairs;
/* Default maximum transmit unit advice */
__u16 mtu;
+   /*
+* speed, in units of 1Mb. All values 0 to INT_MAX are legal.
+* Any other value stands for unknown.
+*/
+   __u32 speed;
+   /*
+* 0x00 - half duplex
+* 0x01 - full duplex
+* Any other value stands for unknown.
+*/
+   __u8 duplex;
 } __attribute__((packed));
 
 /*
-- 
2.6.1

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH net-next 1/2] virtio_net: Introduce VIRTIO_NET_F_MASTER feature bit

[Samudrala, Sridhar]

On 1/3/2018 9:43 AM, Michael S. Tsirkin wrote:

On Tue, Jan 02, 2018 at 04:35:37PM -0800, Sridhar Samudrala wrote:

This feature bit can be used by hypervisor to indicate virtio_net device to
act as a master for a directly attached passthru device with the same MAC
address.

Signed-off-by: Sridhar Samudrala 
---
  drivers/net/virtio_net.c| 3 ++-
  include/uapi/linux/virtio_net.h | 1 +
  2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 6fb7b658a6cc..46844a1d9a62 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -2796,7 +2796,8 @@ static struct virtio_device_id id_table[] = {
VIRTIO_NET_F_CTRL_RX, VIRTIO_NET_F_CTRL_VLAN, \
VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ, \
VIRTIO_NET_F_CTRL_MAC_ADDR, \
-   VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS
+   VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, \
+   VIRTIO_NET_F_MASTER
  
  static unsigned int features[] = {

VIRTNET_FEATURES,
diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
index fc353b518288..a9b4e0836786 100644
--- a/include/uapi/linux/virtio_net.h
+++ b/include/uapi/linux/virtio_net.h
@@ -56,6 +56,7 @@
  #define VIRTIO_NET_F_MQ   22  /* Device supports Receive Flow
 * Steering */
  #define VIRTIO_NET_F_CTRL_MAC_ADDR 23 /* Set MAC address */
+#define VIRTIO_NET_F_MASTER62  /* act as master for a VF device */

Well the virtio hardware doesn't really know whether it's a master or
the slave of a bond. In fact quite possibly down the road we'll add a
virtual device on top as others seem to want to do - that other one will
be the master then.

And we do nothing do check it's a VF.

So maybe a better name would be

#define VIRTIO_NET_F_BACKUP 62  /* Act as backup for another device 
with the same MAC */
Yes. BACKUP  does make more sense as virtio is only used as a BACKUP 
datapath when the other

device is unplugged.




  
  #ifndef VIRTIO_NET_NO_LEGACY

  #define VIRTIO_NET_F_GSO  6   /* Host handles pkts w/ any GSO type */
--
2.14.3


___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH v3 0/3] virtio_net: allow hypervisor to indicate linkspeed and duplex setting

[Jason Baron via Virtualization]

We have found it useful to be able to set the linkspeed and duplex
settings from the host-side for virtio_net. This obviates the need
for guest changes and settings for these fields, and does not require
custom ethtool commands for virtio_net.

The ability to set linkspeed and duplex is useful in various cases
as described here:

16032be virtio_net: add ethtool support for set and get of settings 

  
Using 'ethtool -s' continues to over-write the linkspeed/duplex
settings with this patch.

The 1/3 patch is against net-next, while the 2-3/3 patch are the associated
qemu changes that would go in after as update-linux-headers.sh should
be run first. So the qemu patches are a demonstration of how I intend this
to work.

Thanks,

-Jason  

linux changes:

changes from v2:
* move speed/duplex read into virtnet_config_changed_work() so link up changes
  are detected

Jason Baron (1):
  virtio_net: propagate linkspeed/duplex settings from the hypervisor

 drivers/net/virtio_net.c| 19 ++-
 include/uapi/linux/virtio_net.h | 13 +
 2 files changed, 31 insertions(+), 1 deletion(-)

qemu changes:

changes from v2:
* if link up return configured speed/duplex, else return UNKNOWN speed and 
duplex

Jason Baron (2):
  qemu: virtio-net: use 64-bit values for feature flags
  qemu: add linkspeed and duplex settings to virtio-net

 hw/net/virtio-net.c | 89 -
 include/hw/virtio/virtio-net.h  |  5 +-
 include/standard-headers/linux/virtio_net.h | 13 +
 3 files changed, 79 insertions(+), 28 deletions(-)


-- 
2.6.1

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH net-next 0/2] Enable virtio to act as a master for a passthru device

[Samudrala, Sridhar]

On 1/3/2018 10:28 AM, Alexander Duyck wrote:

On Wed, Jan 3, 2018 at 10:14 AM, Samudrala, Sridhar
 wrote:


On 1/3/2018 8:59 AM, Alexander Duyck wrote:

On Tue, Jan 2, 2018 at 6:16 PM, Jakub Kicinski  wrote:

On Tue,  2 Jan 2018 16:35:36 -0800, Sridhar Samudrala wrote:

This patch series enables virtio to switch over to a VF datapath when a
VF
netdev is present with the same MAC address. It allows live migration of
a VM
with a direct attached VF without the need to setup a bond/team between
a
VF and virtio net device in the guest.

The hypervisor needs to unplug the VF device from the guest on the
source
host and reset the MAC filter of the VF to initiate failover of datapath
to
virtio before starting the migration. After the migration is completed,
the
destination hypervisor sets the MAC filter on the VF and plugs it back
to
the guest to switch over to VF datapath.

It is based on netvsc implementation and it may be possible to make this
code
generic and move it to a common location that can be shared by netvsc
and virtio.

This patch series is based on the discussion initiated by Jesse on this
thread.
https://marc.info/?l=linux-virtualization=151189725224231=2

How does the notion of a device which is both a bond and a leg of a
bond fit with Alex's recent discussions about feature propagation?
Which propagation rules will apply to VirtIO master?  Meaning of the
flags on a software upper device may be different.  Why muddy the
architecture like this and not introduce a synthetic bond device?

It doesn't really fit with the notion I had. I think there may have
been a bit of a disconnect as I have been out for the last week or so
for the holidays.

My thought on this was that the feature bit should be spawning a new
para-virtual bond device and that bond should have the virto and the
VF as slaves. Also I thought there was some discussion about trying to
reuse as much of the netvsc code as possible for this so that we could
avoid duplication of effort and have the two drivers use the same
approach. It seems like it should be pretty straight forward since you
would have the feature bit in the case of virto, and netvsc just does
this sort of thing by default if I am not mistaken.

This patch is mostly based on netvsc implementation. The only change is
avoiding the
explicit dev_open() call of the VF netdev after a delay. I am assuming that
the guest userspace
will bring up the VF netdev and the hypervisor will update the MAC filters
to switch to
the right data path.
We could commonize the code and make it shared between netvsc and virtio. Do
we want
to do this right away or later? If so, what would be a good location for
these shared functions?
Is it net/core/dev.c?

No, I would think about starting a new driver file in "/drivers/net/".
The idea is this driver would be utilized to create a bond
automatically and set the appropriate registration hooks. If nothing
else you could probably just call it something generic like virt-bond
or vbond or whatever.


We are trying to avoid creating another driver or a device.  Can we look 
into

consolidation of the 2 implementations(virtio & netvsc) as a later patch?



Also, if we want to go with a solution that creates a bond device, do we
want virtio_net/netvsc
drivers to create a upper device?  Such a solution is already possible via
config scripts that can
create a bond with virtio and a VF net device as slaves.  netvsc and this
patch series is trying to
make it as simple as possible for the VM to use directly attached devices
and support live migration
by switching to virtio datapath as a backup during the migration process
when the VF device
is unplugged.

We all understand that. But you are making the solution very virtio
specific. We want to see this be usable for other interfaces such as
netsc and whatever other virtual interfaces are floating around out
there.

Also I haven't seen us address what happens as far as how we will
handle this on the host. My thought was we should have a paired
interface. Something like veth, but made up of a bond on each end. So
in the host we should have one bond that has a tap/vhost interface and
a VF port representor, and on the other we would be looking at the
virtio interface and the VF. Attaching the tap/vhost to the bond could
be a way of triggering the feature bit to be set in the virtio. That
way communication between the guest and the host won't get too
confusing as you will see all traffic from the bonded MAC address
always show up on the host side bond instead of potentially showing up
on two unrelated interfaces. It would also make for a good way to
resolve the east/west traffic problem on hosts since you could just
send the broadcast/multicast traffic via the tap/vhost/virtio channel
instead of having to send it back through the port representor and eat
up all that PCIe bus traffic.
From the host point of view, here is a simple script that needs to be 
run to do the

live 

Re: [PATCH net-next 0/2] Enable virtio to act as a master for a passthru device

[Samudrala, Sridhar]

On 1/3/2018 8:59 AM, Alexander Duyck wrote:

On Tue, Jan 2, 2018 at 6:16 PM, Jakub Kicinski  wrote:

On Tue,  2 Jan 2018 16:35:36 -0800, Sridhar Samudrala wrote:

This patch series enables virtio to switch over to a VF datapath when a VF
netdev is present with the same MAC address. It allows live migration of a VM
with a direct attached VF without the need to setup a bond/team between a
VF and virtio net device in the guest.

The hypervisor needs to unplug the VF device from the guest on the source
host and reset the MAC filter of the VF to initiate failover of datapath to
virtio before starting the migration. After the migration is completed, the
destination hypervisor sets the MAC filter on the VF and plugs it back to
the guest to switch over to VF datapath.

It is based on netvsc implementation and it may be possible to make this code
generic and move it to a common location that can be shared by netvsc and 
virtio.

This patch series is based on the discussion initiated by Jesse on this thread.
https://marc.info/?l=linux-virtualization=151189725224231=2

How does the notion of a device which is both a bond and a leg of a
bond fit with Alex's recent discussions about feature propagation?
Which propagation rules will apply to VirtIO master?  Meaning of the
flags on a software upper device may be different.  Why muddy the
architecture like this and not introduce a synthetic bond device?

It doesn't really fit with the notion I had. I think there may have
been a bit of a disconnect as I have been out for the last week or so
for the holidays.

My thought on this was that the feature bit should be spawning a new
para-virtual bond device and that bond should have the virto and the
VF as slaves. Also I thought there was some discussion about trying to
reuse as much of the netvsc code as possible for this so that we could
avoid duplication of effort and have the two drivers use the same
approach. It seems like it should be pretty straight forward since you
would have the feature bit in the case of virto, and netvsc just does
this sort of thing by default if I am not mistaken.
This patch is mostly based on netvsc implementation. The only change is 
avoiding the
explicit dev_open() call of the VF netdev after a delay. I am assuming 
that the guest userspace
will bring up the VF netdev and the hypervisor will update the MAC 
filters to switch to

the right data path.
We could commonize the code and make it shared between netvsc and 
virtio. Do we want
to do this right away or later? If so, what would be a good location for 
these shared functions?

Is it net/core/dev.c?

Also, if we want to go with a solution that creates a bond device, do we 
want virtio_net/netvsc
drivers to create a upper device?  Such a solution is already possible 
via config scripts that can
create a bond with virtio and a VF net device as slaves.  netvsc and 
this patch series is trying to
make it as simple as possible for the VM to use directly attached 
devices and support live migration
by switching to virtio datapath as a backup during the migration process 
when the VF device

is unplugged.

Thanks
Sridhar
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH net-next 1/2] virtio_net: Introduce VIRTIO_NET_F_MASTER feature bit

[Sridhar Samudrala]

This feature bit can be used by hypervisor to indicate virtio_net device to
act as a master for a directly attached passthru device with the same MAC
address.

Signed-off-by: Sridhar Samudrala 
---
 drivers/net/virtio_net.c| 3 ++-
 include/uapi/linux/virtio_net.h | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 6fb7b658a6cc..46844a1d9a62 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -2796,7 +2796,8 @@ static struct virtio_device_id id_table[] = {
VIRTIO_NET_F_CTRL_RX, VIRTIO_NET_F_CTRL_VLAN, \
VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ, \
VIRTIO_NET_F_CTRL_MAC_ADDR, \
-   VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS
+   VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, \
+   VIRTIO_NET_F_MASTER
 
 static unsigned int features[] = {
VIRTNET_FEATURES,
diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
index fc353b518288..a9b4e0836786 100644
--- a/include/uapi/linux/virtio_net.h
+++ b/include/uapi/linux/virtio_net.h
@@ -56,6 +56,7 @@
 #define VIRTIO_NET_F_MQ22  /* Device supports Receive Flow
 * Steering */
 #define VIRTIO_NET_F_CTRL_MAC_ADDR 23  /* Set MAC address */
+#define VIRTIO_NET_F_MASTER62  /* act as master for a VF device */
 
 #ifndef VIRTIO_NET_NO_LEGACY
 #define VIRTIO_NET_F_GSO   6   /* Host handles pkts w/ any GSO type */
-- 
2.14.3

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH net-next 2/2] virtio_net: Extend virtio to use VF datapath when available

[Sridhar Samudrala]

This patch enables virtio to switch over to a VF datapath when a VF netdev
is present with the same MAC address.  It allows live migration of a VM
with a direct attached VF without the need to setup a bond/team between a
VF and virtio net device in the guest.

The hypervisor needs to unplug the VF device from the guest on the source
host and reset the MAC filter of the VF to initiate failover of datapath to
virtio before starting the migration. After the migration is completed, the
destination hypervisor sets the MAC filter on the VF and plugs it back to
the guest to switch over to VF datapath.

This patch is based on the discussion initiated by Jesse on this thread.
https://marc.info/?l=linux-virtualization=151189725224231=2

Signed-off-by: Sridhar Samudrala 
Reviewed-by: Jesse Brandeburg 
---
 drivers/net/virtio_net.c | 305 ++-
 1 file changed, 303 insertions(+), 2 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 46844a1d9a62..6516dcb55b7d 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -31,6 +31,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 
 static int napi_weight = NAPI_POLL_WEIGHT;
 module_param(napi_weight, int, 0444);
@@ -117,6 +119,15 @@ struct receive_queue {
char name[40];
 };
 
+struct virtnet_vf_pcpu_stats {
+   u64 rx_packets;
+   u64 rx_bytes;
+   u64 tx_packets;
+   u64 tx_bytes;
+   struct u64_stats_sync   syncp;
+   u32 tx_dropped;
+};
+
 struct virtnet_info {
struct virtio_device *vdev;
struct virtqueue *cvq;
@@ -179,6 +190,10 @@ struct virtnet_info {
u32 speed;
 
unsigned long guest_offloads;
+
+   /* State to manage the associated VF interface. */
+   struct net_device __rcu *vf_netdev;
+   struct virtnet_vf_pcpu_stats __percpu *vf_stats;
 };
 
 struct padded_vnet_hdr {
@@ -1303,16 +1318,51 @@ static int xmit_skb(struct send_queue *sq, struct 
sk_buff *skb)
return virtqueue_add_outbuf(sq->vq, sq->sg, num_sg, skb, GFP_ATOMIC);
 }
 
+/* Send skb on the slave VF device. */
+static int virtnet_vf_xmit(struct net_device *dev, struct net_device 
*vf_netdev,
+  struct sk_buff *skb)
+{
+   struct virtnet_info *vi = netdev_priv(dev);
+   unsigned int len = skb->len;
+   int rc;
+
+   skb->dev = vf_netdev;
+   skb->queue_mapping = qdisc_skb_cb(skb)->slave_dev_queue_mapping;
+
+   rc = dev_queue_xmit(skb);
+   if (likely(rc == NET_XMIT_SUCCESS || rc == NET_XMIT_CN)) {
+   struct virtnet_vf_pcpu_stats *pcpu_stats
+   = this_cpu_ptr(vi->vf_stats);
+
+   u64_stats_update_begin(_stats->syncp);
+   pcpu_stats->tx_packets++;
+   pcpu_stats->tx_bytes += len;
+   u64_stats_update_end(_stats->syncp);
+   } else {
+   this_cpu_inc(vi->vf_stats->tx_dropped);
+   }
+
+   return rc;
+}
+
 static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
 {
struct virtnet_info *vi = netdev_priv(dev);
int qnum = skb_get_queue_mapping(skb);
struct send_queue *sq = >sq[qnum];
+   struct net_device *vf_netdev;
int err;
struct netdev_queue *txq = netdev_get_tx_queue(dev, qnum);
bool kick = !skb->xmit_more;
bool use_napi = sq->napi.weight;
 
+   /* If VF is present and up then redirect packets
+* called with rcu_read_lock_bh
+*/
+   vf_netdev = rcu_dereference_bh(vi->vf_netdev);
+   if (vf_netdev && netif_running(vf_netdev) && !netpoll_tx_running(dev))
+   return virtnet_vf_xmit(dev, vf_netdev, skb);
+
/* Free up any pending old buffers before queueing new ones. */
free_old_xmit_skbs(sq);
 
@@ -1459,10 +1509,41 @@ static int virtnet_set_mac_address(struct net_device 
*dev, void *p)
return ret;
 }
 
+static void virtnet_get_vf_stats(struct net_device *dev,
+struct virtnet_vf_pcpu_stats *tot)
+{
+   struct virtnet_info *vi = netdev_priv(dev);
+   int i;
+
+   memset(tot, 0, sizeof(*tot));
+
+   for_each_possible_cpu(i) {
+   const struct virtnet_vf_pcpu_stats *stats
+   = per_cpu_ptr(vi->vf_stats, i);
+   u64 rx_packets, rx_bytes, tx_packets, tx_bytes;
+   unsigned int start;
+
+   do {
+   start = u64_stats_fetch_begin_irq(>syncp);
+   rx_packets = stats->rx_packets;
+   tx_packets = stats->tx_packets;
+   rx_bytes = stats->rx_bytes;
+   tx_bytes = stats->tx_bytes;
+   } while (u64_stats_fetch_retry_irq(>syncp, start));
+
+   tot->rx_packets += rx_packets;
+   tot->tx_packets += tx_packets;
+

Re: [PATCH 00/13] remove_conflicting_framebuffers() cleanup

[Bartlomiej Zolnierkiewicz]

On Monday, November 27, 2017 11:30:44 AM Daniel Vetter wrote:
> On Fri, Nov 24, 2017 at 06:53:25PM +0100, Michał Mirosław wrote:
> > This series cleans up duplicated code for replacing firmware FB
> > driver with proper DRI driver and adds handover support to
> > Tegra driver.

Please Cc: me on and linux-fbdev ML on fbdev related patches
(I was Cc:-ed only on the cover letter and patch #10, linux-fbdev
was not Cc:-ed at all).

> > The last patch is here because it uses new semantics of
> > remove_conflicting_framebuffers() from this series. This
> > can be considered independently, though.
> 
> Except for that patches I've commented on:
> 
> Acked-by: Daniel Vetter 
> 
> Since this is for tegra and Thierry has drm-misc commit rights, it's
> probably simplest when Thierry pushes this all to drm-misc once driver
> maintainers had a chance to look at it. Also needs and ack from Bart for
> the fbdev sides.

For fbdev changes:

Acked-by: Bartlomiej Zolnierkiewicz 

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R Institute Poland
Samsung Electronics

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH net-next v2 1/3] virtio_net: propagate linkspeed/duplex settings from the hypervisor

[Jason Baron via Virtualization]

On 12/27/2017 04:43 PM, David Miller wrote:
> From: Jason Baron 
> Date: Fri, 22 Dec 2017 16:54:01 -0500
> 
>> The ability to set speed and duplex for virtio_net in useful in various
>> scenarios as described here:
>>
>> 16032be virtio_net: add ethtool support for set and get of settings
>>
>> However, it would be nice to be able to set this from the hypervisor,
>> such that virtio_net doesn't require custom guest ethtool commands.
>>
>> Introduce a new feature flag, VIRTIO_NET_F_SPEED_DUPLEX, which allows
>> the hypervisor to export a linkspeed and duplex setting. The user can
>> subsequently overwrite it later if desired via: 'ethtool -s'.
>>
>> Signed-off-by: Jason Baron 
>> Cc: "Michael S. Tsirkin" 
>> Cc: Jason Wang 
> 
> Looks mostly fine to me but need some virtio_net reviewers on this one.
> 
>> @@ -57,6 +57,8 @@
>>   * Steering */
>>  #define VIRTIO_NET_F_CTRL_MAC_ADDR 23   /* Set MAC address */
>>  
>> +#define VIRTIO_NET_F_SPEED_DUPLEX 63/* Host set linkspeed and 
>> duplex */
>> +
> 
> Why use a value so far away from the largest existing one?
> 
> Just curious.
> 

So that came from a discussion with Michael about which bit to use for
this, and he suggested using 63:

"
Transports started from bit 24 and are growing up.
So I would say devices should start from bit 63 and grow down.
"

https://patchwork.ozlabs.org/patch/848814/#1826669

I will add a comment to explain it.

Thanks,

-Jason

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH net-next] virtio_net: Add ethtool stats

[Florian Fainelli]

On December 25, 2017 3:45:36 AM PST, Toshiaki Makita 
 wrote:
>On 2017/12/25 3:16, Stephen Hemminger wrote:
>> On Wed, 20 Dec 2017 13:40:37 +0900
>> Toshiaki Makita  wrote:
>> 
>>> +
>>> +static const struct virtnet_gstats virtnet_gstrings_stats[] = {
>>> +   { "rx_packets", VIRTNET_NETDEV_STAT(rx_packets) },
>>> +   { "tx_packets", VIRTNET_NETDEV_STAT(tx_packets) },
>>> +   { "rx_bytes",   VIRTNET_NETDEV_STAT(rx_bytes) },
>>> +   { "tx_bytes",   VIRTNET_NETDEV_STAT(tx_bytes) },
>>> +   { "rx_dropped", VIRTNET_NETDEV_STAT(rx_dropped) },
>>> +   { "rx_length_errors",   VIRTNET_NETDEV_STAT(rx_length_errors) },
>>> +   { "rx_frame_errors",VIRTNET_NETDEV_STAT(rx_frame_errors) },
>>> +   { "tx_dropped", VIRTNET_NETDEV_STAT(tx_dropped) },
>>> +   { "tx_fifo_errors", VIRTNET_NETDEV_STAT(tx_fifo_errors) },
>>> +};
>>> +
>> 
>> Please do not merge pre-existing global stats into ethtool.
>> It just duplicates existing functionality.
>
>Thanks for the feedback.
>I thought it's handy to be able to check stats like this in ethtool as
>well. Some drivers like ixgbe and mlx5 do similar thing.
>I can remove these duplicate stats (unless anyone has objection).

For existing drivers, ethtool stats are kind of user ABI, so once these stats 
get in, we should not consider removing them. For new drivers or existing 
drivers without ethtool stats, we can do things a bit better as Stephen 
suggested.

-- 
Florian
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH net-next 0/2] Enable virtio to act as a master for a passthru device

[Sridhar Samudrala]

This patch series enables virtio to switch over to a VF datapath when a VF
netdev is present with the same MAC address. It allows live migration of a VM
with a direct attached VF without the need to setup a bond/team between a
VF and virtio net device in the guest.

The hypervisor needs to unplug the VF device from the guest on the source
host and reset the MAC filter of the VF to initiate failover of datapath to
virtio before starting the migration. After the migration is completed, the
destination hypervisor sets the MAC filter on the VF and plugs it back to
the guest to switch over to VF datapath.

It is based on netvsc implementation and it may be possible to make this code 
generic and move it to a common location that can be shared by netvsc and 
virtio.

This patch series is based on the discussion initiated by Jesse on this thread.
https://marc.info/?l=linux-virtualization=151189725224231=2

Sridhar Samudrala (2):
  virtio_net: Introduce VIRTIO_NET_F_MASTER feature bit
  virtio_net: Extend virtio to use VF datapath when available

 drivers/net/virtio_net.c| 308 +++-
 include/uapi/linux/virtio_net.h |   1 +
 2 files changed, 306 insertions(+), 3 deletions(-)

-- 
2.14.3

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH] vhost: remove unused lock check flag in vhost_dev_cleanup()

[夷则(Caspar)]

In commit ea5d404655ba ("vhost: fix release path lockdep checks"),
Michael added a flag to check whether we should hold a lock in
vhost_dev_cleanup(), however, in commit 47283bef7ed3 ("vhost: move
memory pointer to VQs"), RCU operations have been replaced by
mutex, we can remove the no-longer-used `locked' parameter now.

Signed-off-by: Caspar Zhang 
---
 drivers/vhost/net.c   | 2 +-
 drivers/vhost/scsi.c  | 2 +-
 drivers/vhost/test.c  | 2 +-
 drivers/vhost/vhost.c | 5 ++---
 drivers/vhost/vhost.h | 2 +-
 drivers/vhost/vsock.c | 2 +-
 6 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index c7bdeb655646..a354d8d731e3 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -996,7 +996,7 @@ static int vhost_net_release(struct inode *inode, struct 
file *f)
vhost_net_stop(n, _sock, _sock);
vhost_net_flush(n);
vhost_dev_stop(>dev);
-   vhost_dev_cleanup(>dev, false);
+   vhost_dev_cleanup(>dev);
vhost_net_vq_reset(n);
if (tx_sock)
sockfd_put(tx_sock);
diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index 71517b3c5558..797d08916553 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -1420,7 +1420,7 @@ static int vhost_scsi_release(struct inode *inode, struct 
file *f)
mutex_unlock(>dev.mutex);
vhost_scsi_clear_endpoint(vs, );
vhost_dev_stop(>dev);
-   vhost_dev_cleanup(>dev, false);
+   vhost_dev_cleanup(>dev);
/* Jobs can re-queue themselves in evt kick handler. Do extra flush. */
vhost_scsi_flush(vs);
kfree(vs->dev.vqs);
diff --git a/drivers/vhost/test.c b/drivers/vhost/test.c
index 3cc98c07dcd3..906b8f0f19f7 100644
--- a/drivers/vhost/test.c
+++ b/drivers/vhost/test.c
@@ -157,7 +157,7 @@ static int vhost_test_release(struct inode *inode, struct 
file *f)
 
vhost_test_stop(n, );
vhost_test_flush(n);
-   vhost_dev_cleanup(>dev, false);
+   vhost_dev_cleanup(>dev);
/* We do an extra flush before freeing memory,
 * since jobs can re-queue themselves. */
vhost_test_flush(n);
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 33ac2b186b85..014675c3d569 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -544,7 +544,7 @@ void vhost_dev_reset_owner(struct vhost_dev *dev, struct 
vhost_umem *umem)
 {
int i;
 
-   vhost_dev_cleanup(dev, true);
+   vhost_dev_cleanup(dev);
 
/* Restore memory to default empty mapping. */
INIT_LIST_HEAD(>umem_list);
@@ -611,8 +611,7 @@ static void vhost_clear_msg(struct vhost_dev *dev)
spin_unlock(>iotlb_lock);
 }
 
-/* Caller should have device mutex if and only if locked is set */
-void vhost_dev_cleanup(struct vhost_dev *dev, bool locked)
+void vhost_dev_cleanup(struct vhost_dev *dev)
 {
int i;
 
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index 79c6e7a60a5e..ff4d918e3e0a 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -181,7 +181,7 @@ bool vhost_dev_has_owner(struct vhost_dev *dev);
 long vhost_dev_check_owner(struct vhost_dev *);
 struct vhost_umem *vhost_dev_reset_owner_prepare(void);
 void vhost_dev_reset_owner(struct vhost_dev *, struct vhost_umem *);
-void vhost_dev_cleanup(struct vhost_dev *, bool locked);
+void vhost_dev_cleanup(struct vhost_dev *);
 void vhost_dev_stop(struct vhost_dev *);
 long vhost_dev_ioctl(struct vhost_dev *, unsigned int ioctl, void __user 
*argp);
 long vhost_vring_ioctl(struct vhost_dev *d, int ioctl, void __user *argp);
diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
index 5a5e981bd8e4..0d14e2ff19f1 100644
--- a/drivers/vhost/vsock.c
+++ b/drivers/vhost/vsock.c
@@ -599,7 +599,7 @@ static int vhost_vsock_dev_release(struct inode *inode, 
struct file *file)
}
spin_unlock_bh(>send_pkt_list_lock);
 
-   vhost_dev_cleanup(>dev, false);
+   vhost_dev_cleanup(>dev);
kfree(vsock->dev.vqs);
vhost_vsock_free(vsock);
return 0;
-- 
2.15.1

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH net-next v2 1/3] virtio_net: propagate linkspeed/duplex settings from the hypervisor

[Jason Baron via Virtualization]

The ability to set speed and duplex for virtio_net in useful in various
scenarios as described here:

16032be virtio_net: add ethtool support for set and get of settings

However, it would be nice to be able to set this from the hypervisor,
such that virtio_net doesn't require custom guest ethtool commands.

Introduce a new feature flag, VIRTIO_NET_F_SPEED_DUPLEX, which allows
the hypervisor to export a linkspeed and duplex setting. The user can
subsequently overwrite it later if desired via: 'ethtool -s'.

Signed-off-by: Jason Baron 
Cc: "Michael S. Tsirkin" 
Cc: Jason Wang 
---
 drivers/net/virtio_net.c| 17 -
 include/uapi/linux/virtio_net.h |  5 +
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 511f833..4168d82 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -2621,6 +2621,20 @@ static int virtnet_probe(struct virtio_device *vdev)
netif_set_real_num_rx_queues(dev, vi->curr_queue_pairs);
 
virtnet_init_settings(dev);
+   if (virtio_has_feature(vdev, VIRTIO_NET_F_SPEED_DUPLEX)) {
+   u32 speed;
+   u8 duplex;
+
+   speed = virtio_cread32(vdev, offsetof(struct virtio_net_config,
+  speed));
+   if (ethtool_validate_speed(speed))
+   vi->speed = speed;
+   duplex = virtio_cread8(vdev,
+  offsetof(struct virtio_net_config,
+  duplex));
+   if (ethtool_validate_duplex(duplex))
+   vi->duplex = duplex;
+   }
 
err = register_netdev(dev);
if (err) {
@@ -2746,7 +2760,8 @@ static struct virtio_device_id id_table[] = {
VIRTIO_NET_F_CTRL_RX, VIRTIO_NET_F_CTRL_VLAN, \
VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ, \
VIRTIO_NET_F_CTRL_MAC_ADDR, \
-   VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS
+   VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, \
+   VIRTIO_NET_F_SPEED_DUPLEX
 
 static unsigned int features[] = {
VIRTNET_FEATURES,
diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
index fc353b5..0f1548e 100644
--- a/include/uapi/linux/virtio_net.h
+++ b/include/uapi/linux/virtio_net.h
@@ -57,6 +57,8 @@
 * Steering */
 #define VIRTIO_NET_F_CTRL_MAC_ADDR 23  /* Set MAC address */
 
+#define VIRTIO_NET_F_SPEED_DUPLEX 63   /* Host set linkspeed and duplex */
+
 #ifndef VIRTIO_NET_NO_LEGACY
 #define VIRTIO_NET_F_GSO   6   /* Host handles pkts w/ any GSO type */
 #endif /* VIRTIO_NET_NO_LEGACY */
@@ -76,6 +78,9 @@ struct virtio_net_config {
__u16 max_virtqueue_pairs;
/* Default maximum transmit unit advice */
__u16 mtu;
+   /* Host exported linkspeed and duplex */
+   __u32 speed;
+   __u8 duplex;
 } __attribute__((packed));
 
 /*
-- 
2.6.1

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH 3/3] qemu: add linkspeed and duplex settings to virtio-net

[Jason Baron via Virtualization]

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=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(+)
 create mode 16 pixman

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index adc20df..7fafe6e 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 = 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)},
 {}
 };
 
@@ -88,6 +96,8 @@ static void virtio_net_get_config(VirtIODevice *vdev, uint8_t 
*config)
 virtio_stw_p(vdev, , n->status);
 virtio_stw_p(vdev, _virtqueue_pairs, n->max_queues);
 virtio_stw_p(vdev, , n->net_conf.mtu);
+virtio_stl_p(vdev, , n->net_conf.speed);
+netcfg.duplex = n->net_conf.duplex;
 memcpy(netcfg.mac, n->mac, ETH_ALEN);
 memcpy(config, , n->config_size);
 }
@@ -1941,6 +1951,23 @@ static void virtio_net_device_realize(DeviceState *dev, 
Error **errp)
 n->host_features |= (1ULL << VIRTIO_NET_F_MTU);
 }
 
+n->host_features |= (1ULL << VIRTIO_NET_F_SPEED_DUPLEX);
+if (n->net_conf.duplex_str) {
+if (strncmp(n->net_conf.duplex_str, "half", 5) == 0) {
+n->net_conf.duplex = DUPLEX_HALF;
+} else if (strncmp(n->net_conf.duplex_str, "full", 5) == 0) {
+n->net_conf.duplex = DUPLEX_FULL;
+} else {
+error_setg(errp, "'duplex' must be 'half' or 'full'");
+}
+} else {
+n->net_conf.duplex = DUPLEX_UNKNOWN;
+}
+if (n->net_conf.speed < SPEED_UNKNOWN) {
+error_setg(errp, "'speed' must be between -1 (SPEED_UNKOWN) and "
+   "INT_MAX");
+}
+
 virtio_net_set_config_size(n, n->host_features);
 virtio_init(vdev, "virtio-net", VIRTIO_ID_NET, n->config_size);
 
@@ -2160,6 +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),
+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 */
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_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_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;
+   /* 

[PATCH 2/3] qemu: use 64-bit values for feature flags in virtio-net

[Jason Baron via Virtualization]

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 
Cc: "Michael S. Tsirkin" 
Cc: Jason Wang 
---
 hw/net/virtio-net.c| 54 +-
 include/hw/virtio/virtio-net.h |  2 +-
 2 files changed, 28 insertions(+), 28 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 38674b0..adc20df 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,45 @@ static const VMStateDescription vmstate_virtio_net = {
 };
 
 static Property virtio_net_properties[] = {
-DEFINE_PROP_BIT("csum", VirtIONet, host_features, VIRTIO_NET_F_CSUM, true),
-DEFINE_PROP_BIT("guest_csum", VirtIONet, host_features,
+DEFINE_PROP_BIT64("csum", VirtIONet, host_features, VIRTIO_NET_F_CSUM, 
true),
+DEFINE_PROP_BIT64("guest_csum", VirtIONet, host_features,
 VIRTIO_NET_F_GUEST_CSUM, true),
-DEFINE_PROP_BIT("gso", VirtIONet, host_features, VIRTIO_NET_F_GSO, true),
-DEFINE_PROP_BIT("guest_tso4", VirtIONet, host_features,
+DEFINE_PROP_BIT64("gso", VirtIONet, host_features, VIRTIO_NET_F_GSO, true),
+DEFINE_PROP_BIT64("guest_tso4", VirtIONet, host_features,
 VIRTIO_NET_F_GUEST_TSO4, true),
-DEFINE_PROP_BIT("guest_tso6", VirtIONet, host_features,
+DEFINE_PROP_BIT64("guest_tso6", VirtIONet, host_features,
 VIRTIO_NET_F_GUEST_TSO6, true),
-DEFINE_PROP_BIT("guest_ecn", VirtIONet, host_features,
+DEFINE_PROP_BIT64("guest_ecn", VirtIONet, host_features,
 VIRTIO_NET_F_GUEST_ECN, true),
-DEFINE_PROP_BIT("guest_ufo", VirtIONet, host_features,
+DEFINE_PROP_BIT64("guest_ufo", VirtIONet, host_features,
 VIRTIO_NET_F_GUEST_UFO, true),
-DEFINE_PROP_BIT("guest_announce", VirtIONet, host_features,
+DEFINE_PROP_BIT64("guest_announce", VirtIONet, host_features,
 VIRTIO_NET_F_GUEST_ANNOUNCE, true),
-DEFINE_PROP_BIT("host_tso4", VirtIONet, host_features,
+DEFINE_PROP_BIT64("host_tso4", VirtIONet, host_features,
 VIRTIO_NET_F_HOST_TSO4, true),
-DEFINE_PROP_BIT("host_tso6", VirtIONet, host_features,
+DEFINE_PROP_BIT64("host_tso6", VirtIONet, host_features,
 VIRTIO_NET_F_HOST_TSO6, true),
-DEFINE_PROP_BIT("host_ecn", VirtIONet, host_features,
+DEFINE_PROP_BIT64("host_ecn", VirtIONet, host_features,
 VIRTIO_NET_F_HOST_ECN, true),
-DEFINE_PROP_BIT("host_ufo", VirtIONet, host_features,
+DEFINE_PROP_BIT64("host_ufo", VirtIONet, host_features,
 VIRTIO_NET_F_HOST_UFO, true),
-DEFINE_PROP_BIT("mrg_rxbuf", VirtIONet, host_features,
+DEFINE_PROP_BIT64("mrg_rxbuf", VirtIONet, host_features,
 VIRTIO_NET_F_MRG_RXBUF, true),
-DEFINE_PROP_BIT("status", VirtIONet, host_features,
+DEFINE_PROP_BIT64("status", VirtIONet, host_features,
 VIRTIO_NET_F_STATUS, true),
-DEFINE_PROP_BIT("ctrl_vq", VirtIONet, host_features,
+DEFINE_PROP_BIT64("ctrl_vq", VirtIONet, host_features,
 VIRTIO_NET_F_CTRL_VQ, true),
-DEFINE_PROP_BIT("ctrl_rx", VirtIONet, host_features,
+DEFINE_PROP_BIT64("ctrl_rx", VirtIONet, host_features,
 VIRTIO_NET_F_CTRL_RX, true),
-DEFINE_PROP_BIT("ctrl_vlan", VirtIONet, host_features,
+DEFINE_PROP_BIT64("ctrl_vlan", VirtIONet, host_features,
 VIRTIO_NET_F_CTRL_VLAN, true),
-DEFINE_PROP_BIT("ctrl_rx_extra", VirtIONet, host_features,
+DEFINE_PROP_BIT64("ctrl_rx_extra", VirtIONet, host_features,
 VIRTIO_NET_F_CTRL_RX_EXTRA, true),
-DEFINE_PROP_BIT("ctrl_mac_addr", VirtIONet, host_features,
+

[PATCH v2 0/3] virtio_net: allow hypervisor to indicate linkspeed and duplex setting

[Jason Baron via Virtualization]

We have found it useful to be able to set the linkspeed and duplex
settings from the host-side for virtio_net. This obviates the need
for guest changes and settings for these fields, and does not require
custom ethtool commands for virtio_net.

  
The ability to set linkspeed and duplex is useful in various cases
as described here:  
  

  
16032be virtio_net: add ethtool support for set and get of settings 
  

  
Using 'ethtool -s' continues to over-write the linkspeed/duplex 
  
settings with this patch.   
   

  
The 1/3 patch is against net-next, while the 2-3/3 patch are the associated
qemu changes that would go in after as update-linux-headers.sh should
be run first. So the qemu patches are a demonstration of how I intend this
to work.

  
Thanks, 
  

  
-Jason  
  

  
 linux changes: 
  

Jason Baron (1):
  virtio_net: propagate linkspeed/duplex settings from the hypervisor

 drivers/net/virtio_net.c| 17 -
 include/uapi/linux/virtio_net.h |  5 +
 2 files changed, 21 insertions(+), 1 deletion(-)

  

  
 qemu changes:  
  

Jason Baron (2):
  qemu: virtio-net: use 64-bit values for feature flags
  qemu: add linkspeed and duplex settings to virtio-net

 hw/net/virtio-net.c | 83 +++--
 include/hw/virtio/virtio-net.h  |  5 +-
 include/standard-headers/linux/virtio_net.h |  4 ++
 3 files changed, 64 insertions(+), 28 deletions(-)
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [RFC PATCH] virtio_net: Extend virtio to use VF datapath when available

[Siwei Liu]

On Wed, Dec 20, 2017 at 8:52 PM, Jakub Kicinski  wrote:
> On Wed, 20 Dec 2017 18:16:30 -0800, Siwei Liu wrote:
>> > The plan is to remove the delay and do the naming in the kernel.
>> > This was suggested by Lennart since udev is only doing naming policy
>> > because kernel names were not repeatable.
>> >
>> > This makes the VF show up as "ethN_vf" on Hyper-V which is user friendly.
>> >
>> > Patch is pending.
>>
>> While it's good to show VF with specific naming to indicate
>> enslavement, I wonder wouldn't it be better to hide this netdev at all
>> from the user space? IMHO this extra device is useless when being
>> enslaved and we may delegate controls (e.g. ethtool) over to the
>> para-virtual device instead? That way it's possible to eliminate the
>> possibility of additional udev setup or modification?
>>
>> I'm not sure if this  is consistent with Windows guest or not, but I
>> don't find it _Linux_ user friendly that ethtool doesn't work on the
>> composite interface any more, and I have to end up with finding out
>> the correct enslaved VF I must operate on.
>
> Hiding "low level" netdevs comes up from time to time, and is more
> widely applicable than just to VF bonds.  We should find a generic
> solution to that problem.

Wholeheartedly agreed.

Be it a generic virtual bond or virtio-net specific one, there should
be some common code between netvsc and virtio for this type of work.
For avoiding duplicated bugs, consistent (Linux) user experience,
future code refactoring/management, and whatever...

One thing worth to note is that, unlike the Hyper-V netvsc backend
there's currently no equivalent (fine-grained) Linux ndo_* driver
interface that is able to move around MAC address/VLAN filters at
run-time specifically. The OID_RECEIVE_FILTER_MOVE_FILTER request I
mean. That translates to one substantial difference in VF
plumbing/unplumbing sequence: you cannot move the MAC address around
to paravirt device until VF is fully unplugged out of the guest OS. I
don't know what backend changes to be proposed for virtio-net as
helper, but the common code needs to work with both flavors of data
path switching backends and do its job correctly.

Regards,
-Siwei
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Question about eliminating virtio-scsi 30s timeout and hot-unplug

[Jason Behmer via Virtualization]

Hi Paolo,
I had a question about this patch that eliminates virtio-scsi timeout on
IOs - https://patchwork.kernel.org/patch/9802047/.  How does this work when
we have IOs outstanding to a device that is then hot-unplugged.  I'm under
the impression that those IOs will never get returned with any status
(please correct me if I'm wrong), and we will then end up waiting on them
forever with this patch.

Thanks,
Jason
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: virtio: make VIRTIO a menuconfig to ease disabling it all

[Andrei Vagin]

On Sat, Dec 09, 2017 at 04:26:57PM +0100, Vincent Legoll wrote:
> No need to get into the submenu to disable all VIRTIO-related
> config entries.
> 
> This makes it easier to disable all VIRTIO config options
> without entering the submenu. It will also enable one
> to see that en/dis-abled state from the outside menu.
> 
> This is only intended to change menuconfig UI, not change
> the config dependencies.
> 
> Signed-off-by: Vincent Legoll 
> ---
>  drivers/virtio/Kconfig | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig
> index cff773f15b7e..d485a63a8233 100644
> --- a/drivers/virtio/Kconfig
> +++ b/drivers/virtio/Kconfig
> @@ -5,7 +5,10 @@ config VIRTIO
> bus, such as CONFIG_VIRTIO_PCI, CONFIG_VIRTIO_MMIO, CONFIG_RPMSG
> or CONFIG_S390_GUEST.
>  
> -menu "Virtio drivers"
> +menuconfig VIRTIO_MENU
> + bool "Virtio drivers"

Hi Vincent,

make localyesconfig and make localmodconfig doesn't work with this
patch.

My scenario looks like this:
* Create a virtual machine with Ubuntu 14.04 in GCE
* git clone git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
* cd linux-next
* curl -o .config 
https://raw.githubusercontent.com/avagin/criu/linux-next/scripts/linux-next-config
* make localyesconfig

Without this patch:

$ cat .config | grep VIRTIO
CONFIG_BLK_MQ_VIRTIO=y
CONFIG_VIRTIO_BLK=y
# CONFIG_VIRTIO_BLK_SCSI is not set
CONFIG_SCSI_VIRTIO=y
CONFIG_VIRTIO_NET=y
CONFIG_VIRTIO_CONSOLE=y
# CONFIG_HW_RANDOM_VIRTIO is not set
CONFIG_VIRTIO=y
CONFIG_VIRTIO_PCI=y
CONFIG_VIRTIO_PCI_LEGACY=y
CONFIG_VIRTIO_BALLOON=y
# CONFIG_VIRTIO_INPUT is not set
CONFIG_VIRTIO_MMIO=y
CONFIG_VIRTIO_MMIO_CMDLINE_DEVICES=y
# CONFIG_RPMSG_VIRTIO is not set
# CONFIG_CRYPTO_DEV_VIRTIO is not set


With this patch:

$ cat .config | grep VIRTIO
CONFIG_BLK_MQ_VIRTIO=y
# CONFIG_VIRTIO_BLK is not set
CONFIG_SCSI_VIRTIO=y
# CONFIG_VIRTIO_NET is not set
CONFIG_CAIF_VIRTIO=y
# CONFIG_VIRTIO_CONSOLE is not set
# CONFIG_HW_RANDOM_VIRTIO is not set
CONFIG_VIRTIO=y
# CONFIG_VIRTIO_MENU is not set
# CONFIG_RPMSG_VIRTIO is not set
# CONFIG_CRYPTO_DEV_VIRTIO is not set


You can see that with this patch CONFIG_VIRTIO_BLK is not set.
It is wrong, and the kernel with this config will not be able to boot.

We can add "default y" to fix this problem.

https://travis-ci.org/avagin/linux/jobs/313348334
https://travis-ci.org/avagin/linux/jobs/318491188

Thanks,
Andrei

> +
> +if VIRTIO_MENU
>  
>  config VIRTIO_PCI
>   tristate "PCI driver for virtio devices"
> @@ -79,4 +82,4 @@ config VIRTIO_MMIO_CMDLINE_DEVICES
>  
>If unsure, say 'N'.
>  
> -endmenu
> +endif # VIRTIO_MENU
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [RFC PATCH] virtio_net: Extend virtio to use VF datapath when available

[Siwei Liu]

On Tue, Dec 19, 2017 at 10:41 AM, Stephen Hemminger
 wrote:
> On Tue, 19 Dec 2017 13:21:17 -0500 (EST)
> David Miller  wrote:
>
>> From: Stephen Hemminger 
>> Date: Tue, 19 Dec 2017 09:55:48 -0800
>>
>> > could be 10ms, just enough to let udev do its renaming
>>
>> Please, move to some kind of notification or event based handling of
>> this problem.
>>
>> No delay is safe, what if userspace gets swapped out or whatever
>> else might make userspace stall unexpectedly?
>>
>
> The plan is to remove the delay and do the naming in the kernel.
> This was suggested by Lennart since udev is only doing naming policy
> because kernel names were not repeatable.
>
> This makes the VF show up as "ethN_vf" on Hyper-V which is user friendly.
>
> Patch is pending.

While it's good to show VF with specific naming to indicate
enslavement, I wonder wouldn't it be better to hide this netdev at all
from the user space? IMHO this extra device is useless when being
enslaved and we may delegate controls (e.g. ethtool) over to the
para-virtual device instead? That way it's possible to eliminate the
possibility of additional udev setup or modification?

I'm not sure if this  is consistent with Windows guest or not, but I
don't find it _Linux_ user friendly that ethtool doesn't work on the
composite interface any more, and I have to end up with finding out
the correct enslaved VF I must operate on.

Regards,
-Siwei
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [RFC PATCH] virtio_net: Extend virtio to use VF datapath when available

[Siwei Liu]

On Tue, Dec 19, 2017 at 10:41 AM, Stephen Hemminger <
step...@networkplumber.org> wrote:

> On Tue, 19 Dec 2017 13:21:17 -0500 (EST)
> David Miller  wrote:
>
> > From: Stephen Hemminger 
> > Date: Tue, 19 Dec 2017 09:55:48 -0800
> >
> > > could be 10ms, just enough to let udev do its renaming
> >
> > Please, move to some kind of notification or event based handling of
> > this problem.
> >
> > No delay is safe, what if userspace gets swapped out or whatever
> > else might make userspace stall unexpectedly?
> >
>
> The plan is to remove the delay and do the naming in the kernel.
> This was suggested by Lennart since udev is only doing naming policy
> because kernel names were not repeatable.
>
> This makes the VF show up as "ethN_vf" on Hyper-V which is user friendly.
>
> Patch is pending.
>

While it's good to show VF with specific naming to indicate enslavement, I
wonder wouldn't it be better to hide this netdev at all from the user
space? IMHO this extra device is useless when being enslaved and we may
delegate controls (e.g. ethtool) over to the para-virtual device instead?
That way it's possible to eliminate the possibility of additional udev
setup or modification?

I'm not sure if this  is consistent with Windows guest or not, but I don't
find it _Linux_ user friendly that ethtool doesn't work on the composite
interface any more, and I have to end up with finding out the correct
enslaved VF I must operate on.

Regards,
-Siwei
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [RFC PATCH] virtio_net: Extend virtio to use VF datapath when available

[Samudrala, Sridhar]

On 12/19/2017 2:53 PM, Stephen Hemminger wrote:

On Tue, 19 Dec 2017 14:37:50 -0800
"Samudrala, Sridhar"  wrote:


On 12/19/2017 11:46 AM, Stephen Hemminger wrote:

On Tue, 19 Dec 2017 11:42:33 -0800
"Samudrala, Sridhar"  wrote:
  

On 12/19/2017 10:41 AM, Stephen Hemminger wrote:

On Tue, 19 Dec 2017 13:21:17 -0500 (EST)
David Miller  wrote:
 

From: Stephen Hemminger 
Date: Tue, 19 Dec 2017 09:55:48 -0800
 

could be 10ms, just enough to let udev do its renaming

Please, move to some kind of notification or event based handling of
this problem.

No delay is safe, what if userspace gets swapped out or whatever
else might make userspace stall unexpectedly?
 

The plan is to remove the delay and do the naming in the kernel.
This was suggested by Lennart since udev is only doing naming policy
because kernel names were not repeatable.

This makes the VF show up as "ethN_vf" on Hyper-V which is user friendly.

Patch is pending.

Do we really need to delay the setup until the name is changed?
Can't we call dev_set_mtu() and dev_open() until dev_change_name() is done?

Thanks
Sridhar

You can call dev_set_mtu, but when dev_open is done the device name
can not be changed by userspace.

I did a quick test to remove the delay and also the dev_open() call and
i don't see
any issues with virtio taking over the VF datapath.
Only the netdev_info() messages may show old device name.

Any specific scenario where we need to explicitly call  VF's dev_open()
in the VF setup process?
I tried i40evf driver loaded after virtio_net  AND  virtio_net loading
after i40evf.


It happens with hotplug. It is possible on Hyper-V to hotplug SR-IOV on
and off while guest is running. If SR-IOV is disabled in host then the
VF device is removed (hotplug) and the inverse. If the master device is
up then the VF device should be brought up by the master device.


Even with KVM, we need to hot unplug SR-IOV device on the source host 
and  plug it
back on the destination host to enable live migration. It all works for 
me even without
the dev_open() of the lower device  from the VF setup routine. Will send 
out a v2 version
of this patch without the delayed VF setup after some more testing next 
week.


Thanks
Sridhar

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [RFC PATCH] virtio_net: Extend virtio to use VF datapath when available

[Samudrala, Sridhar]

On 12/19/2017 11:46 AM, Stephen Hemminger wrote:

On Tue, 19 Dec 2017 11:42:33 -0800
"Samudrala, Sridhar"  wrote:


On 12/19/2017 10:41 AM, Stephen Hemminger wrote:

On Tue, 19 Dec 2017 13:21:17 -0500 (EST)
David Miller  wrote:
  

From: Stephen Hemminger 
Date: Tue, 19 Dec 2017 09:55:48 -0800
  

could be 10ms, just enough to let udev do its renaming

Please, move to some kind of notification or event based handling of
this problem.

No delay is safe, what if userspace gets swapped out or whatever
else might make userspace stall unexpectedly?
  

The plan is to remove the delay and do the naming in the kernel.
This was suggested by Lennart since udev is only doing naming policy
because kernel names were not repeatable.

This makes the VF show up as "ethN_vf" on Hyper-V which is user friendly.

Patch is pending.

Do we really need to delay the setup until the name is changed?
Can't we call dev_set_mtu() and dev_open() until dev_change_name() is done?

Thanks
Sridhar

You can call dev_set_mtu, but when dev_open is done the device name
can not be changed by userspace.
I did a quick test to remove the delay and also the dev_open() call and 
i don't see

any issues with virtio taking over the VF datapath.
Only the netdev_info() messages may show old device name.

Any specific scenario where we need to explicitly call  VF's dev_open() 
in the VF setup process?
I tried i40evf driver loaded after virtio_net  AND  virtio_net loading 
after i40evf.


Thanks
Sridhar
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [RFC PATCH] virtio_net: Extend virtio to use VF datapath when available

[Samudrala, Sridhar]

On 12/19/2017 10:41 AM, Stephen Hemminger wrote:

On Tue, 19 Dec 2017 13:21:17 -0500 (EST)
David Miller  wrote:


From: Stephen Hemminger 
Date: Tue, 19 Dec 2017 09:55:48 -0800


could be 10ms, just enough to let udev do its renaming

Please, move to some kind of notification or event based handling of
this problem.

No delay is safe, what if userspace gets swapped out or whatever
else might make userspace stall unexpectedly?


The plan is to remove the delay and do the naming in the kernel.
This was suggested by Lennart since udev is only doing naming policy
because kernel names were not repeatable.

This makes the VF show up as "ethN_vf" on Hyper-V which is user friendly.

Patch is pending.

Do we really need to delay the setup until the name is changed?
Can't we call dev_set_mtu() and dev_open() until dev_change_name() is done?

Thanks
Sridhar
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [RFC PATCH] virtio_net: Extend virtio to use VF datapath when available

[Samudrala, Sridhar]

On 12/19/2017 7:47 AM, Michael S. Tsirkin wrote:

I'll need to look at this more, in particular the feature
bit is missing here. For now one question:

On Mon, Dec 18, 2017 at 04:40:36PM -0800, Sridhar Samudrala wrote:

@@ -56,6 +58,8 @@ module_param(napi_tx, bool, 0644);
   */
  DECLARE_EWMA(pkt_len, 0, 64)
  
+#define VF_TAKEOVER_INT	(HZ / 10)

+
  #define VIRTNET_DRIVER_VERSION "1.0.0"
  
  static const unsigned long guest_offloads[] = {

Why is this delay necessary? And why by 100ms?


This is based on netvsc implementation and here is the commit that
added this delay.  Not sure if this needs to be 100ms.

commit 6123c66854c174e4982f98195100c1d990f9e5e6
Author: stephen hemminger 
Date:   Wed Aug 9 17:46:03 2017 -0700

    netvsc: delay setup of VF device

    When VF device is discovered, delay bring it automatically up in
    order to allow userspace to some simple changes (like renaming).



___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[RFC PATCH] virtio_net: Extend virtio to use VF datapath when available

[Sridhar Samudrala]

This patch enables virtio to switch over to a VF datapath when a VF netdev
is present with the same MAC address.  It allows live migration of a VM
with a direct attached VF without the need to setup a bond/team between a
VF and virtio net device in the guest.

The hypervisor needs to unplug the VF device from the guest on the source
host and reset the MAC filter of the VF to initiate failover of datapath to
virtio before starting the migration. After the migration is completed, the
destination hypervisor sets the MAC filter on the VF and plugs it back to
the guest to switch over to VF datapath.

It is entirely based on netvsc implementation and it should be possible to
make this code generic and move it to a common location that can be shared
by netvsc and virtio.

Also, i think we should make this a negotiated feature that is off by
default via a new feature bit.

This patch is based on the discussion initiated by Jesse on this thread.
https://marc.info/?l=linux-virtualization=151189725224231=2

Signed-off-by: Sridhar Samudrala 
Reviewed-by: Jesse Brandeburg 
---
 drivers/net/virtio_net.c | 341 ++-
 1 file changed, 339 insertions(+), 2 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 559b215c0169..a34c717bb15b 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -31,6 +31,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 
 static int napi_weight = NAPI_POLL_WEIGHT;
 module_param(napi_weight, int, 0444);
@@ -56,6 +58,8 @@ module_param(napi_tx, bool, 0644);
  */
 DECLARE_EWMA(pkt_len, 0, 64)
 
+#define VF_TAKEOVER_INT(HZ / 10)
+
 #define VIRTNET_DRIVER_VERSION "1.0.0"
 
 static const unsigned long guest_offloads[] = {
@@ -117,6 +121,15 @@ struct receive_queue {
char name[40];
 };
 
+struct virtnet_vf_pcpu_stats {
+   u64 rx_packets;
+   u64 rx_bytes;
+   u64 tx_packets;
+   u64 tx_bytes;
+   struct u64_stats_sync   syncp;
+   u32 tx_dropped;
+};
+
 struct virtnet_info {
struct virtio_device *vdev;
struct virtqueue *cvq;
@@ -179,6 +192,11 @@ struct virtnet_info {
u32 speed;
 
unsigned long guest_offloads;
+
+   /* State to manage the associated VF interface. */
+   struct net_device __rcu *vf_netdev;
+   struct virtnet_vf_pcpu_stats __percpu *vf_stats;
+   struct delayed_work vf_takeover;
 };
 
 struct padded_vnet_hdr {
@@ -1300,16 +1318,51 @@ static int xmit_skb(struct send_queue *sq, struct 
sk_buff *skb)
return virtqueue_add_outbuf(sq->vq, sq->sg, num_sg, skb, GFP_ATOMIC);
 }
 
+/* Send skb on the slave VF device. */
+static int virtnet_vf_xmit(struct net_device *dev, struct net_device 
*vf_netdev,
+  struct sk_buff *skb)
+{
+   struct virtnet_info *vi = netdev_priv(dev);
+   unsigned int len = skb->len;
+   int rc;
+
+   skb->dev = vf_netdev;
+   skb->queue_mapping = qdisc_skb_cb(skb)->slave_dev_queue_mapping;
+
+   rc = dev_queue_xmit(skb);
+   if (likely(rc == NET_XMIT_SUCCESS || rc == NET_XMIT_CN)) {
+   struct virtnet_vf_pcpu_stats *pcpu_stats
+   = this_cpu_ptr(vi->vf_stats);
+
+   u64_stats_update_begin(_stats->syncp);
+   pcpu_stats->tx_packets++;
+   pcpu_stats->tx_bytes += len;
+   u64_stats_update_end(_stats->syncp);
+   } else {
+   this_cpu_inc(vi->vf_stats->tx_dropped);
+   }
+
+   return rc;
+}
+
 static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
 {
struct virtnet_info *vi = netdev_priv(dev);
int qnum = skb_get_queue_mapping(skb);
struct send_queue *sq = >sq[qnum];
+   struct net_device *vf_netdev;
int err;
struct netdev_queue *txq = netdev_get_tx_queue(dev, qnum);
bool kick = !skb->xmit_more;
bool use_napi = sq->napi.weight;
 
+   /* if VF is present and up then redirect packets
+* called with rcu_read_lock_bh
+*/
+   vf_netdev = rcu_dereference_bh(vi->vf_netdev);
+   if (vf_netdev && netif_running(vf_netdev) && !netpoll_tx_running(dev))
+   return virtnet_vf_xmit(dev, vf_netdev, skb);
+
/* Free up any pending old buffers before queueing new ones. */
free_old_xmit_skbs(sq);
 
@@ -1456,10 +1509,41 @@ static int virtnet_set_mac_address(struct net_device 
*dev, void *p)
return ret;
 }
 
+static void virtnet_get_vf_stats(struct net_device *dev,
+struct virtnet_vf_pcpu_stats *tot)
+{
+   struct virtnet_info *vi = netdev_priv(dev);
+   int i;
+
+   memset(tot, 0, sizeof(*tot));
+
+   for_each_possible_cpu(i) {
+   const struct virtnet_vf_pcpu_stats *stats
+   = per_cpu_ptr(vi->vf_stats, i);
+   u64 rx_packets, rx_bytes, 

CFP SECRYPT 2018 - 15th Int.l Conf. on Security and Cryptography (Porto/Portugal)

[ic...@insticc.info]

SUBMISSION DEADLINE 

15th International Conference on Security and Cryptography

Submission Deadline: March 13, 2018

http://www.secrypt.icete.org/

July 26 - 28, 2018
Porto, Portugal.

 


 
A short list of presented papers will be selected so that revised and extended 
versions of these papers will be published by Springer.
 
All papers presented at the congress venue will also be available at the 
SCITEPRESS Digital Library (http://www.scitepress.org/DigitalLibrary/).
  
Should you have any question please don’t hesitate contacting me.
 

Kind regards,
SECRYPT Secretariat

Address: Av. D. Manuel I, 27A, 2º esq.
2910-595 Setubal, Portugal
Tel: +351 265 520 185
Fax: +351 265 520 186
Web: http://www.secrypt.icete.org/
e-mail: secrypt.secretar...@insticc.org

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

CFP ICETE 2018 - 15th Int.l Joint Conf. on e-Business and Telecommunications (Porto/Portugal)

[ic...@insticc.info]

SUBMISSION DEADLINE 

15th International Joint Conference on e-Business and Telecommunications

Submission Deadline: March 13, 2018

http://www.icete.org/

July 26 - 28, 2018
Porto, Portugal.

 


 
A short list of presented papers will be selected so that revised and extended 
versions of these papers will be published by Springer.
 
All papers presented at the congress venue will also be available at the 
SCITEPRESS Digital Library (http://www.scitepress.org/DigitalLibrary/).
  
Should you have any question please don’t hesitate contacting me.
 

Kind regards,
ICETE Secretariat

Address: Av. D. Manuel I, 27A, 2º esq.
2910-595 Setubal, Portugal
Tel: +351 265 520 185
Fax: +351 265 520 186
Web: http://www.icete.org/
e-mail: icete.secretar...@insticc.org

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

CFP WINSYS 2018 - 15th Int.l Conf. on Wireless Networks and Mobile Systems (Porto/Portugal)

[ic...@insticc.info]

SUBMISSION DEADLINE 

15th International Conference on Wireless Networks and Mobile Systems

Submission Deadline: March 13, 2018

http://www.winsys.icete.org/

July 26 - 28, 2018
Porto, Portugal.

 WINSYS is organized in 3 major tracks:

 - Sensor Networks and Ad Hoc Communications
 - Wireless and Mobile Technologies
 - Mobile Software and Services


 
A short list of presented papers will be selected so that revised and extended 
versions of these papers will be published by Springer.
 
All papers presented at the congress venue will also be available at the 
SCITEPRESS Digital Library (http://www.scitepress.org/DigitalLibrary/).
  
Should you have any question please don’t hesitate contacting me.
 

Kind regards,
WINSYS Secretariat

Address: Av. D. Manuel I, 27A, 2º esq.
2910-595 Setubal, Portugal
Tel: +351 265 520 185
Fax: +351 265 520 186
Web: http://www.winsys.icete.org/
e-mail: winsys.secretar...@insticc.org

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

CFP DCNET 2018 - 9th Int.l Conf. on Data Communication Networking (Porto/Portugal)

[ic...@insticc.info]

SUBMISSION DEADLINE 

9th International Conference on Data Communication Networking

Submission Deadline: March 13, 2018

http://www.dcnet.icete.org/

July 26 - 28, 2018
Porto, Portugal.

 


 
A short list of presented papers will be selected so that revised and extended 
versions of these papers will be published by Springer.
 
All papers presented at the congress venue will also be available at the 
SCITEPRESS Digital Library (http://www.scitepress.org/DigitalLibrary/).
  
Should you have any question please don’t hesitate contacting me.
 

Kind regards,
DCNET Secretariat

Address: Av. D. Manuel I, 27A, 2º esq.
2910-595 Setubal, Portugal
Tel: +351 265 520 185
Fax: +351 265 520 186
Web: http://www.dcnet.icete.org/
e-mail: dcnet.secretar...@insticc.org

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 6/6] crypto: stm32-cryp: convert to the new crypto engine API

[Fabien DESSENNE]

On 03/01/18 21:11, Corentin Labbe wrote:
> This patch convert the stm32-cryp driver to the new crypto engine API.
> Signed-off-by: Corentin Labbe 
> ---
>   drivers/crypto/stm32/stm32-cryp.c | 21 -
>   1 file changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/crypto/stm32/stm32-cryp.c 
> b/drivers/crypto/stm32/stm32-cryp.c
> index cf1dddbeaa2c..99e0473ef247 100644
> --- a/drivers/crypto/stm32/stm32-cryp.c
> +++ b/drivers/crypto/stm32/stm32-cryp.c
> @@ -91,6 +91,7 @@
>   #define _walked_out (cryp->out_walk.offset - 
> cryp->out_sg->offset)
>   
>   struct stm32_cryp_ctx {
> + struct crypto_engine_reqctx enginectx;
>   struct stm32_cryp   *cryp;
>   int keylen;
>   u32 key[AES_KEYSIZE_256 / sizeof(u32)];
> @@ -494,10 +495,20 @@ static int stm32_cryp_cpu_start(struct stm32_cryp *cryp)
>   return 0;
>   }
>   
> +static int stm32_cryp_cipher_one_req(struct crypto_engine *engine,
> +  void *areq);

Merge these 2 lines in a single one

> +static int stm32_cryp_prepare_cipher_req(struct crypto_engine *engine,
> +  void *areq);
> +
>   static int stm32_cryp_cra_init(struct crypto_tfm *tfm)
>   {
> + struct stm32_cryp_ctx *ctx = crypto_tfm_ctx(tfm);
> +
>   tfm->crt_ablkcipher.reqsize = sizeof(struct stm32_cryp_reqctx);
>   
> + ctx->enginectx.op.do_one_request = stm32_cryp_cipher_one_req;
> + ctx->enginectx.op.prepare_request = stm32_cryp_prepare_cipher_req;
> + ctx->enginectx.op.unprepare_request = NULL;
>   return 0;
>   }
>   
> @@ -695,14 +706,17 @@ static int stm32_cryp_prepare_req(struct crypto_engine 
> *engine,
>   }
>   
>   static int stm32_cryp_prepare_cipher_req(struct crypto_engine *engine,
> -  struct ablkcipher_request *req)
> +  void *areq)
>   {
> + struct ablkcipher_request *req = container_of(areq, struct 
> ablkcipher_request, base);

 > 80 characters (CHECKPATCH)

> +
>   return stm32_cryp_prepare_req(engine, req);
>   }
>   
>   static int stm32_cryp_cipher_one_req(struct crypto_engine *engine,
> -  struct ablkcipher_request *req)
> +  void *areq)

Merge these 2 lines in a single one

>   {
> + struct ablkcipher_request *req = container_of(areq, struct 
> ablkcipher_request, base);

 > 80 characters (CHECKPATCH)

>   struct stm32_cryp_ctx *ctx = crypto_ablkcipher_ctx(
>   crypto_ablkcipher_reqtfm(req));
>   struct stm32_cryp *cryp = ctx->cryp;
> @@ -1104,9 +1118,6 @@ static int stm32_cryp_probe(struct platform_device 
> *pdev)
>   goto err_engine1;
>   }
>   
> - cryp->engine->prepare_cipher_request = stm32_cryp_prepare_cipher_req;
> - cryp->engine->cipher_one_request = stm32_cryp_cipher_one_req;
> -
>   ret = crypto_engine_start(cryp->engine);
>   if (ret) {
>   dev_err(dev, "Could not start crypto engine\n");
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 5/6] crypto: stm32-hash: convert to the new crypto engine API

[Fabien DESSENNE]

On 03/01/18 21:11, Corentin Labbe wrote:
> This patch convert the stm32-hash driver to the new crypto engine API.
>
> Signed-off-by: Corentin Labbe 
> ---
>   drivers/crypto/stm32/stm32-hash.c | 18 +-
>   1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/crypto/stm32/stm32-hash.c 
> b/drivers/crypto/stm32/stm32-hash.c
> index 4ca4a264a833..9790c2c936c7 100644
> --- a/drivers/crypto/stm32/stm32-hash.c
> +++ b/drivers/crypto/stm32/stm32-hash.c
> @@ -122,6 +122,7 @@ enum stm32_hash_data_format {
>   #define HASH_DMA_THRESHOLD  50
>   
>   struct stm32_hash_ctx {
> + struct crypto_engine_reqctx enginectx;
>   struct stm32_hash_dev   *hdev;
>   unsigned long   flags;
>   
> @@ -828,6 +829,11 @@ static int stm32_hash_hw_init(struct stm32_hash_dev 
> *hdev,
>   return 0;
>   }
>   
> +static int stm32_hash_one_request(struct crypto_engine *engine,
> +   void *areq);

merge these two lines in a single one

> +static int stm32_hash_prepare_req(struct crypto_engine *engine,
> +   void *areq);

merge these two lines in a single one

> +
>   static int stm32_hash_handle_queue(struct stm32_hash_dev *hdev,
>  struct ahash_request *req)
>   {
> @@ -835,8 +841,9 @@ static int stm32_hash_handle_queue(struct stm32_hash_dev 
> *hdev,
>   }
>   
>   static int stm32_hash_prepare_req(struct crypto_engine *engine,
> -   struct ahash_request *req)
> +   void *areq)

merge these two lines in a single one

>   {
> + struct ahash_request *req = container_of(areq, struct ahash_request, 
> base);

 > 80 characters (CHECKPATCH)

>   struct stm32_hash_ctx *ctx = crypto_ahash_ctx(crypto_ahash_reqtfm(req));
>   struct stm32_hash_dev *hdev = stm32_hash_find_dev(ctx);
>   struct stm32_hash_request_ctx *rctx;
> @@ -855,8 +862,9 @@ static int stm32_hash_prepare_req(struct crypto_engine 
> *engine,
>   }
>   
>   static int stm32_hash_one_request(struct crypto_engine *engine,
> -   struct ahash_request *req)
> +   void *areq)

merge these two lines in a single one

>   {
> + struct ahash_request *req = container_of(areq, struct ahash_request, 
> base);

 > 80 characters (CHECKPATCH)

>   struct stm32_hash_ctx *ctx = crypto_ahash_ctx(crypto_ahash_reqtfm(req));
>   struct stm32_hash_dev *hdev = stm32_hash_find_dev(ctx);
>   struct stm32_hash_request_ctx *rctx;
> @@ -1033,6 +1041,9 @@ static int stm32_hash_cra_init_algs(struct crypto_tfm 
> *tfm,
>   if (algs_hmac_name)
>   ctx->flags |= HASH_FLAGS_HMAC;
>   
> + ctx->enginectx.op.do_one_request = stm32_hash_one_request;
> + ctx->enginectx.op.prepare_request = stm32_hash_prepare_req;
> + ctx->enginectx.op.unprepare_request = NULL;
>   return 0;
>   }
>   
> @@ -1493,9 +1504,6 @@ static int stm32_hash_probe(struct platform_device 
> *pdev)
>   goto err_engine;
>   }
>   
> - hdev->engine->prepare_hash_request = stm32_hash_prepare_req;
> - hdev->engine->hash_one_request = stm32_hash_one_request;
> -
>   ret = crypto_engine_start(hdev->engine);
>   if (ret)
>   goto err_engine_start;
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 2/6] crypto: engine - Permit to enqueue all async requests

[Fabien DESSENNE]

On 03/01/18 21:11, Corentin Labbe wrote:
> The crypto engine could actually only enqueue hash and ablkcipher request.
> This patch permit it to enqueue any type of crypto_async_request.
>
> Signed-off-by: Corentin Labbe 
> ---
>   crypto/crypto_engine.c  | 230 
> 
>   include/crypto/engine.h |  59 +++--
>   2 files changed, 148 insertions(+), 141 deletions(-)
>
> diff --git a/crypto/crypto_engine.c b/crypto/crypto_engine.c
> index 61e7c4e02fd2..036270b61648 100644
> --- a/crypto/crypto_engine.c
> +++ b/crypto/crypto_engine.c
> @@ -15,7 +15,6 @@
>   #include 
>   #include 
>   #include 
> -#include 
>   #include 
>   #include "internal.h"
>   
> @@ -34,11 +33,10 @@ static void crypto_pump_requests(struct crypto_engine 
> *engine,
>bool in_kthread)
>   {
>   struct crypto_async_request *async_req, *backlog;
> - struct ahash_request *hreq;
> - struct ablkcipher_request *breq;
>   unsigned long flags;
>   bool was_busy = false;
> - int ret, rtype;
> + int ret;
> + struct crypto_engine_reqctx *enginectx;
>   
>   spin_lock_irqsave(>queue_lock, flags);
>   
> @@ -94,7 +92,6 @@ static void crypto_pump_requests(struct crypto_engine 
> *engine,
>   
>   spin_unlock_irqrestore(>queue_lock, flags);
>   
> - rtype = crypto_tfm_alg_type(engine->cur_req->tfm);
>   /* Until here we get the request need to be encrypted successfully */
>   if (!was_busy && engine->prepare_crypt_hardware) {
>   ret = engine->prepare_crypt_hardware(engine);
> @@ -104,57 +101,31 @@ static void crypto_pump_requests(struct crypto_engine 
> *engine,
>   }
>   }
>   
> - switch (rtype) {
> - case CRYPTO_ALG_TYPE_AHASH:
> - hreq = ahash_request_cast(engine->cur_req);
> - if (engine->prepare_hash_request) {
> - ret = engine->prepare_hash_request(engine, hreq);
> - if (ret) {
> - dev_err(engine->dev, "failed to prepare 
> request: %d\n",
> - ret);
> - goto req_err;
> - }
> - engine->cur_req_prepared = true;
> - }
> - ret = engine->hash_one_request(engine, hreq);
> - if (ret) {
> - dev_err(engine->dev, "failed to hash one request from 
> queue\n");
> - goto req_err;
> - }
> - return;
> - case CRYPTO_ALG_TYPE_ABLKCIPHER:
> - breq = ablkcipher_request_cast(engine->cur_req);
> - if (engine->prepare_cipher_request) {
> - ret = engine->prepare_cipher_request(engine, breq);
> - if (ret) {
> - dev_err(engine->dev, "failed to prepare 
> request: %d\n",
> - ret);
> - goto req_err;
> - }
> - engine->cur_req_prepared = true;
> - }
> - ret = engine->cipher_one_request(engine, breq);
> + enginectx = crypto_tfm_ctx(async_req->tfm);
> +
> + if (enginectx->op.prepare_request) {
> + ret = enginectx->op.prepare_request(engine, async_req);
>   if (ret) {
> - dev_err(engine->dev, "failed to cipher one request from 
> queue\n");
> + dev_err(engine->dev, "failed to prepare request: %d\n",
> + ret);
>   goto req_err;
>   }
> - return;
> - default:
> - dev_err(engine->dev, "failed to prepare request of unknown 
> type\n");
> - return;
> + engine->cur_req_prepared = true;
> + }
> + if (!enginectx->op.do_one_request) {
> + dev_err(engine->dev, "failed to do request\n");
> + ret = -EINVAL;
> + goto req_err;
> + }
> + ret = enginectx->op.do_one_request(engine, async_req);
> + if (ret) {
> + dev_err(engine->dev, "Failed to do one request from queue: 
> %d\n", ret);
> + goto req_err;
>   }
> + return;
>   
>   req_err:
> - switch (rtype) {
> - case CRYPTO_ALG_TYPE_AHASH:
> - hreq = ahash_request_cast(engine->cur_req);
> - crypto_finalize_hash_request(engine, hreq, ret);
> - break;
> - case CRYPTO_ALG_TYPE_ABLKCIPHER:
> - breq = ablkcipher_request_cast(engine->cur_req);
> - crypto_finalize_cipher_request(engine, breq, ret);
> - break;
> - }
> + crypto_finalize_request(engine, async_req, ret);
>   return;
>   
>   out:
> @@ -170,13 +141,12 @@ static void crypto_pump_work(struct kthread_work *work)
>   }
>   
>   /**
> - * crypto_transfer_cipher_request - transfer the new request into the
> - * enginequeue
> + 

Re: [PATCH 1/6] Documentation: crypto: document crypto engine API

[Fabien DESSENNE]

Hi Corentin,


Thank you for this new version which I have testes successfully with the 
stm32 hash & cryp drivers.

As a general comment on this patchset, I would say that it does not 
cover all async requests: typically I need (for the pending stm32 cryp 
driver uprade) to use CryptoEngine to process AEAD requests which is not 
covered here.

Could you please consider adding the 'transfer' and 'finalize' EXPORTed 
functions for aead requests? (the implementation is quite trivial)

Have also a look at struct acomp_req (acompress.h) and struct 
kpp_request (kpp.h) which also use "struct crypto_async_request base"


BR

Fabien


On 03/01/18 21:11, Corentin Labbe wrote:
> Signed-off-by: Corentin Labbe 
> ---
>   Documentation/crypto/crypto_engine.rst | 46 
> ++
>   1 file changed, 46 insertions(+)
>   create mode 100644 Documentation/crypto/crypto_engine.rst
>
> diff --git a/Documentation/crypto/crypto_engine.rst 
> b/Documentation/crypto/crypto_engine.rst
> new file mode 100644
> index ..b0ed37f9fb0c
> --- /dev/null
> +++ b/Documentation/crypto/crypto_engine.rst
> @@ -0,0 +1,46 @@
> +=
> +CRYPTO ENGINE
> +=
> +
> +Overview
> +
> +The crypto engine API (CE), is a crypto queue manager.
> +
> +Requirement
> +---
> +You have to put at start of your tfm_ctx the struct crypto_engine_reqctx
> +struct your_tfm_ctx {
> +struct crypto_engine_reqctx enginectx;
> +...
> +};
> +Why: Since CE manage only crypto_async_request, it cannot know the underlying
> +request_type and so have access only on the TFM.
> +So using container_of for accessing __ctx is impossible.
> +Furthermore, the crypto engine cannot know the "struct your_tfm_ctx",
> +so it must assume that crypto_engine_reqctx is at start of it.
> +
> +Order of operations
> +---
> +You have to obtain a struct crypto_engine via crypto_engine_alloc_init().
> +And start it via crypto_engine_start().
> +
> +Before transferring any request, you have to fill the enginectx.
> +- prepare_request: (taking a function pointer) If you need to do some 
> processing before doing the request
> +- unprepare_request: (taking a function pointer) Undoing what's done in 
> prepare_request
> +- do_one_request: (taking a function pointer) Do encryption for current 
> request
> +
> +Note: that those three functions get the crypto_async_request associated 
> with the received request.
> +So your need to get the original request via container_of(areq, struct 
> yourrequesttype_request, base);
> +
> +When your driver receive a crypto_request, you have to transfer it to
> +the cryptoengine via one of:
> +- crypto_transfer_cipher_request_to_engine()
> +- crypto_transfer_skcipher_request_to_engine()
> +- crypto_transfer_akcipher_request_to_engine()
> +- crypto_transfer_hash_request_to_engine()
> +
> +At the end of the request process, a call to one of the following function 
> is needed:
> +- crypto_finalize_cipher_request
> +- crypto_finalize_skcipher_request
> +- crypto_finalize_akcipher_request
> +- crypto_finalize_hash_request
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v21 2/5 RESEND] virtio-balloon: VIRTIO_BALLOON_F_SG

[Wei Wang]

On 01/09/2018 10:42 PM, Tetsuo Handa wrote:

Wei Wang wrote:

- enable OOM to free inflated pages maintained in the local temporary
   list.

I do want to see it before applying this patch.



Fine, then what do you think of the method I shared in your post here: 
https://patchwork.kernel.org/patch/10140731/


Michael, could we merge patch 3-5 first?




Please carefully check how the xbitmap implementation works, and you will
find that you are adding a lot of redundant operations with a bug.


This version mainly added some test cases, and it passes the test run 
without any issue. Appreciate it if your comments could be more 
specific, that would make the discussion more effective, for example, I 
deliberately added "xb_find_set(xb1, 2, ULONG_MAX - 3)" for the overflow 
test, not sure if this is the "bug" you referred to, but I'm glad to 
hear your different thought.


I agree that some tests may be repeated in some degree, since we test 
the implementation from different aspects, for example, 
xbitmap_check_bit_range() may have already performed xb_zero() while we 
specifically have another xbitmap_check_zero_bits() which may test 
something that has already been tested when checking bit range. But I 
think testing twice is better than omission.
Also, I left the "Regualr test1: node=NULL" case though the new 
implementation doesn't explicitly use "node" as before, but that 
node=NULL is still a radix tree implementation internally and that case 
looks special to me, so maybe not bad to cover in the test.


You are also welcome to send a patch to remove the redundant one if you 
think that's an issue. Thanks.


Best,
Wei
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization