[PATCH v3] virtio_pmem: fix sparse warning

2019-07-11 Thread Pankaj Gupta
This patch fixes below sparse warning related to __virtio
type in virtio pmem driver. This is reported by Intel test
bot on linux-next tree.

nd_virtio.c:56:28: warning: incorrect type in assignment
(different base types)
nd_virtio.c:56:28:expected unsigned int [unsigned] [usertype] type
nd_virtio.c:56:28:got restricted __virtio32
nd_virtio.c:93:59: warning: incorrect type in argument 2
(different base types)
nd_virtio.c:93:59:expected restricted __virtio32 [usertype] val
nd_virtio.c:93:59:got unsigned int [unsigned] [usertype] ret

Reported-by: kbuild test robot 
Signed-off-by: Pankaj Gupta 
---
This fixes a warning, so submitting it as a separate
patch on top of virtio pmem series.

v2-> v3
 Use __le for req/resp fields - Michael

 drivers/nvdimm/nd_virtio.c   | 4 ++--
 include/uapi/linux/virtio_pmem.h | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/nvdimm/nd_virtio.c b/drivers/nvdimm/nd_virtio.c
index 8645275c08c2..10351d5b49fa 100644
--- a/drivers/nvdimm/nd_virtio.c
+++ b/drivers/nvdimm/nd_virtio.c
@@ -53,7 +53,7 @@ static int virtio_pmem_flush(struct nd_region *nd_region)
init_waitqueue_head(&req_data->host_acked);
init_waitqueue_head(&req_data->wq_buf);
INIT_LIST_HEAD(&req_data->list);
-   req_data->req.type = cpu_to_virtio32(vdev, VIRTIO_PMEM_REQ_TYPE_FLUSH);
+   req_data->req.type = cpu_to_le32(VIRTIO_PMEM_REQ_TYPE_FLUSH);
sg_init_one(&sg, &req_data->req, sizeof(req_data->req));
sgs[0] = &sg;
sg_init_one(&ret, &req_data->resp.ret, sizeof(req_data->resp));
@@ -90,7 +90,7 @@ static int virtio_pmem_flush(struct nd_region *nd_region)
} else {
/* A host repsonse results in "host_ack" getting called */
wait_event(req_data->host_acked, req_data->done);
-   err = virtio32_to_cpu(vdev, req_data->resp.ret);
+   err = le32_to_cpu(req_data->resp.ret);
}
 
kfree(req_data);
diff --git a/include/uapi/linux/virtio_pmem.h b/include/uapi/linux/virtio_pmem.h
index efcd72f2d20d..9a63ed6d062f 100644
--- a/include/uapi/linux/virtio_pmem.h
+++ b/include/uapi/linux/virtio_pmem.h
@@ -23,12 +23,12 @@ struct virtio_pmem_config {
 
 struct virtio_pmem_resp {
/* Host return status corresponding to flush request */
-   __u32 ret;
+   __le32 ret;
 };
 
 struct virtio_pmem_req {
/* command type */
-   __u32 type;
+   __le32 type;
 };
 
 #endif
-- 
2.14.5

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


Re: [PATCH] scsi: virtio_scsi: Use struct_size() helper

2019-07-11 Thread Martin K. Petersen


Gustavo,

> One of the more common cases of allocation size calculations is finding
> the size of a structure that has a zero-sized array at the end, along
> with memory for some number of elements for that array. For example:

Applied to 5.4/scsi-queue, thanks!

-- 
Martin K. Petersen  Oracle Linux Engineering
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH v5 1/8] s390/mm: force swiotlb for protected virtualization

2019-07-11 Thread Thiago Jung Bauermann


Hello Halil,

Halil Pasic  writes:

> On s390, protected virtualization guests have to use bounced I/O
> buffers.  That requires some plumbing.
>
> Let us make sure, any device that uses DMA API with direct ops correctly
> is spared from the problems, that a hypervisor attempting I/O to a
> non-shared page would bring.
>
> Signed-off-by: Halil Pasic 
> Reviewed-by: Claudio Imbrenda 
> ---
>  arch/s390/Kconfig   |  4 +++
>  arch/s390/include/asm/mem_encrypt.h | 18 +++
>  arch/s390/mm/init.c | 47 +
>  3 files changed, 69 insertions(+)
>  create mode 100644 arch/s390/include/asm/mem_encrypt.h
>
> diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
> index 109243fdb6ec..88d8355b7bf7 100644
> --- a/arch/s390/Kconfig
> +++ b/arch/s390/Kconfig
> @@ -1,4 +1,7 @@
>  # SPDX-License-Identifier: GPL-2.0
> +config ARCH_HAS_MEM_ENCRYPT
> +def_bool y
> +
>  config MMU
>   def_bool y
>  

ARCH_HAS_MEM_ENCRYPT is already defined in arch/Kconfig, so I think you
can just select it in config S390 like you do with SWIOTLB.

> @@ -187,6 +190,7 @@ config S390
>   select VIRT_CPU_ACCOUNTING
>   select ARCH_HAS_SCALED_CPUTIME
>   select HAVE_NMI
> + select SWIOTLB
>  
>  
>  config SCHED_OMIT_FRAME_POINTER

-- 
Thiago Jung Bauermann
IBM Linux Technology Center
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [RFC] virtio-net: share receive_*() and add_recvbuf_*() with virtio-vsock

2019-07-11 Thread Michael S. Tsirkin
On Thu, Jul 11, 2019 at 01:41:34PM +0200, Stefano Garzarella wrote:
> On Thu, Jul 11, 2019 at 03:37:00PM +0800, Jason Wang wrote:
> > 
> > On 2019/7/10 下午11:37, Stefano Garzarella wrote:
> > > Hi,
> > > as Jason suggested some months ago, I looked better at the virtio-net 
> > > driver to
> > > understand if we can reuse some parts also in the virtio-vsock driver, 
> > > since we
> > > have similar challenges (mergeable buffers, page allocation, small
> > > packets, etc.).
> > > 
> > > Initially, I would add the skbuff in the virtio-vsock in order to re-use
> > > receive_*() functions.
> > 
> > 
> > Yes, that will be a good step.
> > 
> 
> Okay, I'll go on this way.
> 
> > 
> > > Then I would move receive_[small, big, mergeable]() and
> > > add_recvbuf_[small, big, mergeable]() outside of virtio-net driver, in 
> > > order to
> > > call them also from virtio-vsock. I need to do some refactoring (e.g. 
> > > leave the
> > > XDP part on the virtio-net driver), but I think it is feasible.
> > > 
> > > The idea is to create a virtio-skb.[h,c] where put these functions and a 
> > > new
> > > object where stores some attributes needed (e.g. hdr_len ) and status 
> > > (e.g.
> > > some fields of struct receive_queue).
> > 
> > 
> > My understanding is we could be more ambitious here. Do you see any blocker
> > for reusing virtio-net directly? It's better to reuse not only the functions
> > but also the logic like NAPI to avoid re-inventing something buggy and
> > duplicated.
> > 
> 
> These are my concerns:
> - virtio-vsock is not a "net_device", so a lot of code related to
>   ethtool, net devices (MAC address, MTU, speed, VLAN, XDP, offloading) will 
> be
>   not used by virtio-vsock.
> 
> - virtio-vsock has a different header. We can consider it as part of
>   virtio_net payload, but it precludes the compatibility with old hosts. This
>   was one of the major doubts that made me think about using only the
>   send/recv skbuff functions, that it shouldn't break the compatibility.
> 
> > 
> > > This is an idea of virtio-skb.h that
> > > I have in mind:
> > >  struct virtskb;
> > 
> > 
> > What fields do you want to store in virtskb? It looks to be exist sk_buff is
> > flexible enough to us?
> 
> My idea is to store queues information, like struct receive_queue or
> struct send_queue, and some device attributes (e.g. hdr_len ).
> 
> > 
> > 
> > > 
> > >  struct sk_buff *virtskb_receive_small(struct virtskb *vs, ...);
> > >  struct sk_buff *virtskb_receive_big(struct virtskb *vs, ...);
> > >  struct sk_buff *virtskb_receive_mergeable(struct virtskb *vs, ...);
> > > 
> > >  int virtskb_add_recvbuf_small(struct virtskb*vs, ...);
> > >  int virtskb_add_recvbuf_big(struct virtskb *vs, ...);
> > >  int virtskb_add_recvbuf_mergeable(struct virtskb *vs, ...);
> > > 
> > > For the Guest->Host path it should be easier, so maybe I can add a
> > > "virtskb_send(struct virtskb *vs, struct sk_buff *skb)" with a part of 
> > > the code
> > > of xmit_skb().
> > 
> > 
> > I may miss something, but I don't see any thing that prevents us from using
> > xmit_skb() directly.
> > 
> 
> Yes, but my initial idea was to make it more parametric and not related to the
> virtio_net_hdr, so the 'hdr_len' could be a parameter and the
> 'num_buffers' should be handled by the caller.
> 
> > 
> > > 
> > > Let me know if you have in mind better names or if I should put these 
> > > function
> > > in another place.
> > > 
> > > I would like to leave the control part completely separate, so, for 
> > > example,
> > > the two drivers will negotiate the features independently and they will 
> > > call
> > > the right virtskb_receive_*() function based on the negotiation.
> > 
> > 
> > If it's one the issue of negotiation, we can simply change the
> > virtnet_probe() to deal with different devices.
> > 
> > 
> > > 
> > > I already started to work on it, but before to do more steps and send an 
> > > RFC
> > > patch, I would like to hear your opinion.
> > > Do you think that makes sense?
> > > Do you see any issue or a better solution?
> > 
> > 
> > I still think we need to seek a way of adding some codes on virtio-net.c
> > directly if there's no huge different in the processing of TX/RX. That would
> > save us a lot time.
> 
> After the reading of the buffers from the virtqueue I think the process
> is slightly different, because virtio-net will interface with the network
> stack, while virtio-vsock will interface with the vsock-core (socket).
> So the virtio-vsock implements the following:
> - control flow mechanism to avoid to loose packets, informing the peer
>   about the amount of memory available in the receive queue using some
>   fields in the virtio_vsock_hdr
> - de-multiplexing parsing the virtio_vsock_hdr and choosing the right
>   socket depending on the port
> - socket state handling
> 
> We can use the virtio-net as transport, but we should add a lot of
> code to skip "net device" stuff when it is used by the

Re: [PATCH v2] virtio_pmem: fix sparse warning

2019-07-11 Thread Pankaj Gupta


> 
> On Wed, Jul 10, 2019 at 11:28:32PM +0530, Pankaj Gupta wrote:
> > This patch fixes below sparse warning related to __virtio
> > type in virtio pmem driver. This is reported by Intel test
> > bot on linux-next tree.
> > 
> > nd_virtio.c:56:28: warning: incorrect type in assignment
> > (different base types)
> > nd_virtio.c:56:28:expected unsigned int [unsigned] [usertype] type
> > nd_virtio.c:56:28:got restricted __virtio32
> > nd_virtio.c:93:59: warning: incorrect type in argument 2
> > (different base types)
> > nd_virtio.c:93:59:expected restricted __virtio32 [usertype] val
> > nd_virtio.c:93:59:got unsigned int [unsigned] [usertype] ret
> > 
> > Reported-by: kbuild test robot 
> > Signed-off-by: Pankaj Gupta 
> > ---
> > 
> > This fixes a warning, so submitting it as a separate
> > patch on top of virtio pmem series.
> > 
> >  include/uapi/linux/virtio_pmem.h | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/include/uapi/linux/virtio_pmem.h
> > b/include/uapi/linux/virtio_pmem.h
> > index efcd72f2d20d..7a7435281362 100644
> > --- a/include/uapi/linux/virtio_pmem.h
> > +++ b/include/uapi/linux/virtio_pmem.h
> > @@ -10,7 +10,7 @@
> >  #ifndef _UAPI_LINUX_VIRTIO_PMEM_H
> >  #define _UAPI_LINUX_VIRTIO_PMEM_H
> >  
> > -#include 
> > +#include 
> >  #include 
> >  #include 
> >  
> > @@ -23,12 +23,12 @@ struct virtio_pmem_config {
> >  
> >  struct virtio_pmem_resp {
> > /* Host return status corresponding to flush request */
> > -   __u32 ret;
> > +   __virtio32 ret;
> >  };
> >  
> >  struct virtio_pmem_req {
> > /* command type */
> > -   __u32 type;
> > +   __virtio32 type;
> >  };
> >  
> >  #endif
> 
> Same comment as previously: pls use __le and fix accessors.

Now, I think I got it. 

__virtio is for legacy devices to solve the endianess. 
By default virtio 1.0 and later are little endian.
Will send updated patch soon.

Thank you,
Pankaj

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


Re: [PATCH] virtio_pmem: fix sparse warning

2019-07-11 Thread Pankaj Gupta



> 
> On Wed, Jul 10, 2019 at 07:57:00PM +0530, Pankaj Gupta wrote:
> > This patch fixes below sparse warning related to __virtio
> > type in virtio pmem driver. This is reported by Intel test
> > bot on linux-next tree.
> > 
> > nd_virtio.c:56:28: warning: incorrect type in assignment (different base
> > types)
> > nd_virtio.c:56:28:expected unsigned int [unsigned] [usertype] type
> > nd_virtio.c:56:28:got restricted __virtio32
> > nd_virtio.c:93:59: warning: incorrect type in argument 2 (different base
> > types)
> > nd_virtio.c:93:59:expected restricted __virtio32 [usertype] val
> > nd_virtio.c:93:59:got unsigned int [unsigned] [usertype] ret
> > 
> > Signed-off-by: Pankaj Gupta 
> > Reported-by: kbuild test robot 
> > ---
> > 
> > This fixes a warning, so submitting it as a separate
> > patch on top of virtio pmem series.
> >  
> >  include/uapi/linux/virtio_pmem.h | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/include/uapi/linux/virtio_pmem.h
> > b/include/uapi/linux/virtio_pmem.h
> > index efcd72f2d20d..f89129bf1f84 100644
> > --- a/include/uapi/linux/virtio_pmem.h
> > +++ b/include/uapi/linux/virtio_pmem.h
> > @@ -23,12 +23,12 @@ struct virtio_pmem_config {
> >  
> >  struct virtio_pmem_resp {
> > /* Host return status corresponding to flush request */
> > -   __u32 ret;
> > +   __virtio32 ret;
> >  };
> >  
> >  struct virtio_pmem_req {
> > /* command type */
> > -   __u32 type;
> > +   __virtio32 type;
> >  };
> >  
> >  #endif
> 
> req/resp are in memory right?
> Then this looks like a wrong fix.
> The accessors should all use cpu_to/from_le
> and they types should be __le32.

o.k

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


Re: [PATCH] virtio_pmem: fix sparse warning

2019-07-11 Thread Michael S. Tsirkin
On Wed, Jul 10, 2019 at 07:57:00PM +0530, Pankaj Gupta wrote:
> This patch fixes below sparse warning related to __virtio 
> type in virtio pmem driver. This is reported by Intel test
> bot on linux-next tree.
> 
> nd_virtio.c:56:28: warning: incorrect type in assignment (different base 
> types)
> nd_virtio.c:56:28:expected unsigned int [unsigned] [usertype] type
> nd_virtio.c:56:28:got restricted __virtio32
> nd_virtio.c:93:59: warning: incorrect type in argument 2 (different base 
> types)
> nd_virtio.c:93:59:expected restricted __virtio32 [usertype] val
> nd_virtio.c:93:59:got unsigned int [unsigned] [usertype] ret
> 
> Signed-off-by: Pankaj Gupta 
> Reported-by: kbuild test robot 
> ---
> 
> This fixes a warning, so submitting it as a separate
> patch on top of virtio pmem series. 
>  
>  include/uapi/linux/virtio_pmem.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/uapi/linux/virtio_pmem.h 
> b/include/uapi/linux/virtio_pmem.h
> index efcd72f2d20d..f89129bf1f84 100644
> --- a/include/uapi/linux/virtio_pmem.h
> +++ b/include/uapi/linux/virtio_pmem.h
> @@ -23,12 +23,12 @@ struct virtio_pmem_config {
>  
>  struct virtio_pmem_resp {
>   /* Host return status corresponding to flush request */
> - __u32 ret;
> + __virtio32 ret;
>  };
>  
>  struct virtio_pmem_req {
>   /* command type */
> - __u32 type;
> + __virtio32 type;
>  };
>  
>  #endif

req/resp are in memory right?
Then this looks like a wrong fix.
The accessors should all use cpu_to/from_le
and they types should be __le32.

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


Re: [PATCH v2] virtio_pmem: fix sparse warning

2019-07-11 Thread Michael S. Tsirkin
On Wed, Jul 10, 2019 at 11:28:32PM +0530, Pankaj Gupta wrote:
> This patch fixes below sparse warning related to __virtio
> type in virtio pmem driver. This is reported by Intel test
> bot on linux-next tree.
> 
> nd_virtio.c:56:28: warning: incorrect type in assignment 
>   (different base types)
> nd_virtio.c:56:28:expected unsigned int [unsigned] [usertype] type
> nd_virtio.c:56:28:got restricted __virtio32
> nd_virtio.c:93:59: warning: incorrect type in argument 2 
>   (different base types)
> nd_virtio.c:93:59:expected restricted __virtio32 [usertype] val
> nd_virtio.c:93:59:got unsigned int [unsigned] [usertype] ret
> 
> Reported-by: kbuild test robot 
> Signed-off-by: Pankaj Gupta 
> ---
> 
> This fixes a warning, so submitting it as a separate
> patch on top of virtio pmem series.
> 
>  include/uapi/linux/virtio_pmem.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/include/uapi/linux/virtio_pmem.h 
> b/include/uapi/linux/virtio_pmem.h
> index efcd72f2d20d..7a7435281362 100644
> --- a/include/uapi/linux/virtio_pmem.h
> +++ b/include/uapi/linux/virtio_pmem.h
> @@ -10,7 +10,7 @@
>  #ifndef _UAPI_LINUX_VIRTIO_PMEM_H
>  #define _UAPI_LINUX_VIRTIO_PMEM_H
>  
> -#include 
> +#include 
>  #include 
>  #include 
>  
> @@ -23,12 +23,12 @@ struct virtio_pmem_config {
>  
>  struct virtio_pmem_resp {
>   /* Host return status corresponding to flush request */
> - __u32 ret;
> + __virtio32 ret;
>  };
>  
>  struct virtio_pmem_req {
>   /* command type */
> - __u32 type;
> + __virtio32 type;
>  };
>  
>  #endif

Same comment as previously: pls use __le and fix accessors.

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


Re: [RFC] virtio-net: share receive_*() and add_recvbuf_*() with virtio-vsock

2019-07-11 Thread Stefano Garzarella
On Thu, Jul 11, 2019 at 03:37:00PM +0800, Jason Wang wrote:
> 
> On 2019/7/10 下午11:37, Stefano Garzarella wrote:
> > Hi,
> > as Jason suggested some months ago, I looked better at the virtio-net 
> > driver to
> > understand if we can reuse some parts also in the virtio-vsock driver, 
> > since we
> > have similar challenges (mergeable buffers, page allocation, small
> > packets, etc.).
> > 
> > Initially, I would add the skbuff in the virtio-vsock in order to re-use
> > receive_*() functions.
> 
> 
> Yes, that will be a good step.
> 

Okay, I'll go on this way.

> 
> > Then I would move receive_[small, big, mergeable]() and
> > add_recvbuf_[small, big, mergeable]() outside of virtio-net driver, in 
> > order to
> > call them also from virtio-vsock. I need to do some refactoring (e.g. leave 
> > the
> > XDP part on the virtio-net driver), but I think it is feasible.
> > 
> > The idea is to create a virtio-skb.[h,c] where put these functions and a new
> > object where stores some attributes needed (e.g. hdr_len ) and status (e.g.
> > some fields of struct receive_queue).
> 
> 
> My understanding is we could be more ambitious here. Do you see any blocker
> for reusing virtio-net directly? It's better to reuse not only the functions
> but also the logic like NAPI to avoid re-inventing something buggy and
> duplicated.
> 

These are my concerns:
- virtio-vsock is not a "net_device", so a lot of code related to
  ethtool, net devices (MAC address, MTU, speed, VLAN, XDP, offloading) will be
  not used by virtio-vsock.

- virtio-vsock has a different header. We can consider it as part of
  virtio_net payload, but it precludes the compatibility with old hosts. This
  was one of the major doubts that made me think about using only the
  send/recv skbuff functions, that it shouldn't break the compatibility.

> 
> > This is an idea of virtio-skb.h that
> > I have in mind:
> >  struct virtskb;
> 
> 
> What fields do you want to store in virtskb? It looks to be exist sk_buff is
> flexible enough to us?

My idea is to store queues information, like struct receive_queue or
struct send_queue, and some device attributes (e.g. hdr_len ).

> 
> 
> > 
> >  struct sk_buff *virtskb_receive_small(struct virtskb *vs, ...);
> >  struct sk_buff *virtskb_receive_big(struct virtskb *vs, ...);
> >  struct sk_buff *virtskb_receive_mergeable(struct virtskb *vs, ...);
> > 
> >  int virtskb_add_recvbuf_small(struct virtskb*vs, ...);
> >  int virtskb_add_recvbuf_big(struct virtskb *vs, ...);
> >  int virtskb_add_recvbuf_mergeable(struct virtskb *vs, ...);
> > 
> > For the Guest->Host path it should be easier, so maybe I can add a
> > "virtskb_send(struct virtskb *vs, struct sk_buff *skb)" with a part of the 
> > code
> > of xmit_skb().
> 
> 
> I may miss something, but I don't see any thing that prevents us from using
> xmit_skb() directly.
> 

Yes, but my initial idea was to make it more parametric and not related to the
virtio_net_hdr, so the 'hdr_len' could be a parameter and the
'num_buffers' should be handled by the caller.

> 
> > 
> > Let me know if you have in mind better names or if I should put these 
> > function
> > in another place.
> > 
> > I would like to leave the control part completely separate, so, for example,
> > the two drivers will negotiate the features independently and they will call
> > the right virtskb_receive_*() function based on the negotiation.
> 
> 
> If it's one the issue of negotiation, we can simply change the
> virtnet_probe() to deal with different devices.
> 
> 
> > 
> > I already started to work on it, but before to do more steps and send an RFC
> > patch, I would like to hear your opinion.
> > Do you think that makes sense?
> > Do you see any issue or a better solution?
> 
> 
> I still think we need to seek a way of adding some codes on virtio-net.c
> directly if there's no huge different in the processing of TX/RX. That would
> save us a lot time.

After the reading of the buffers from the virtqueue I think the process
is slightly different, because virtio-net will interface with the network
stack, while virtio-vsock will interface with the vsock-core (socket).
So the virtio-vsock implements the following:
- control flow mechanism to avoid to loose packets, informing the peer
  about the amount of memory available in the receive queue using some
  fields in the virtio_vsock_hdr
- de-multiplexing parsing the virtio_vsock_hdr and choosing the right
  socket depending on the port
- socket state handling

We can use the virtio-net as transport, but we should add a lot of
code to skip "net device" stuff when it is used by the virtio-vsock.
This could break something in virtio-net, for this reason, I thought to reuse
only the send/recv functions starting from the idea to split the virtio-net
driver in two parts:
a. one with all stuff related to the network stack
b. one with the stuff needed to communicate with the host

And use skbuff to communicate between parts. In this way

Re: [RFC] virtio-net: share receive_*() and add_recvbuf_*() with virtio-vsock

2019-07-11 Thread Jason Wang


On 2019/7/10 下午11:37, Stefano Garzarella wrote:

Hi,
as Jason suggested some months ago, I looked better at the virtio-net driver to
understand if we can reuse some parts also in the virtio-vsock driver, since we
have similar challenges (mergeable buffers, page allocation, small
packets, etc.).

Initially, I would add the skbuff in the virtio-vsock in order to re-use
receive_*() functions.



Yes, that will be a good step.



Then I would move receive_[small, big, mergeable]() and
add_recvbuf_[small, big, mergeable]() outside of virtio-net driver, in order to
call them also from virtio-vsock. I need to do some refactoring (e.g. leave the
XDP part on the virtio-net driver), but I think it is feasible.

The idea is to create a virtio-skb.[h,c] where put these functions and a new
object where stores some attributes needed (e.g. hdr_len ) and status (e.g.
some fields of struct receive_queue).



My understanding is we could be more ambitious here. Do you see any 
blocker for reusing virtio-net directly? It's better to reuse not only 
the functions but also the logic like NAPI to avoid re-inventing 
something buggy and duplicated.




This is an idea of virtio-skb.h that
I have in mind:
 struct virtskb;



What fields do you want to store in virtskb? It looks to be exist 
sk_buff is flexible enough to us?





 struct sk_buff *virtskb_receive_small(struct virtskb *vs, ...);
 struct sk_buff *virtskb_receive_big(struct virtskb *vs, ...);
 struct sk_buff *virtskb_receive_mergeable(struct virtskb *vs, ...);

 int virtskb_add_recvbuf_small(struct virtskb*vs, ...);
 int virtskb_add_recvbuf_big(struct virtskb *vs, ...);
 int virtskb_add_recvbuf_mergeable(struct virtskb *vs, ...);

For the Guest->Host path it should be easier, so maybe I can add a
"virtskb_send(struct virtskb *vs, struct sk_buff *skb)" with a part of the code
of xmit_skb().



I may miss something, but I don't see any thing that prevents us from 
using xmit_skb() directly.





Let me know if you have in mind better names or if I should put these function
in another place.

I would like to leave the control part completely separate, so, for example,
the two drivers will negotiate the features independently and they will call
the right virtskb_receive_*() function based on the negotiation.



If it's one the issue of negotiation, we can simply change the 
virtnet_probe() to deal with different devices.





I already started to work on it, but before to do more steps and send an RFC
patch, I would like to hear your opinion.
Do you think that makes sense?
Do you see any issue or a better solution?



I still think we need to seek a way of adding some codes on virtio-net.c 
directly if there's no huge different in the processing of TX/RX. That 
would save us a lot time.


Thanks




Thanks in advance,
Stefano

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