[dpdk-dev] [PATCH v4 7/8] virtio/lib:add virtio guest offload capabilities

2015-11-11 Thread Yuanhan Liu
On Wed, Nov 11, 2015 at 08:53:08AM +, Liu, Jijiang wrote:
...
> > If so, you'd better quote them here, or even to your patch commit.
> 
> In the Virtual I/O Device (VIRTIO) Version 1.0 Committee Specification 02, 
> 
> (1) VIRTIO_NET_F_GUEST_CSUM (1) Driver handles packets with partial checksum.
> 
> (2) If the VIRTIO_NET_F_GUEST_CSUM feature was negotiated, the 
> VIRTIO_NET_HDR_F_NEEDS_-
> CSUM bit in flags MAY be set: if so, the checksum on the packet is incomplete 
> and csum_start
> and csum_offset indicate how to calculate it (see Packet Transmission point 
> 1).
> 
> (3) If the VIRTIO_NET_F_GUEST_TSO4, TSO6 or UFO options were negotiated, then 
> gso_type
> MAY be something other than VIRTIO_NET_HDR_GSO_NONE, and gso_size field 
> indicates
> the desired MSS (see Packet Transmission point 2).

Thanks. Yes, sound like we need set them.

--yliu
> 
> The information can be included in the next version of patch set.
> 
> > --yliu
> > 
> > > Yes, I'd like to listen other guys comments.
> > >
> > > > Hopefully the virtio expert, Michael, could shine some lights on that.
> > > >
> > > > --yliu
> > > >
> > > > >
> > > > >  static uint64_t VHOST_FEATURES = VHOST_SUPPORTED_FEATURES;
> > > > >
> > > > > --
> > > > > 1.7.7.6


[dpdk-dev] [PATCH v4 7/8] virtio/lib:add virtio guest offload capabilities

2015-11-11 Thread Yuanhan Liu
On Wed, Nov 11, 2015 at 08:38:29AM +, Liu, Jijiang wrote:
> 
> 
> > -Original Message-
> > From: Yuanhan Liu [mailto:yuanhan.liu at linux.intel.com]
> > Sent: Wednesday, November 11, 2015 4:31 PM
> > To: Liu, Jijiang; Michael S. Tsirkin
> > Cc: dev at dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH v4 7/8] virtio/lib:add virtio guest offload
> > capabilities
> > 
> > On Wed, Nov 11, 2015 at 02:40:45PM +0800, Jijiang Liu wrote:
> > > Add virtio guest offload capabilities.
> > >
> > > Signed-off-by: Jijiang Liu 
> > > ---
> > >  lib/librte_vhost/virtio-net.c |5 -
> > >  1 files changed, 4 insertions(+), 1 deletions(-)
> > >
> > > diff --git a/lib/librte_vhost/virtio-net.c
> > > b/lib/librte_vhost/virtio-net.c index 81bd309..839a333 100644
> > > --- a/lib/librte_vhost/virtio-net.c
> > > +++ b/lib/librte_vhost/virtio-net.c
> > > @@ -80,7 +80,10 @@ static struct virtio_net_config_ll *ll_root;
> > >   (1ULL <<
> > VHOST_USER_F_PROTOCOL_FEATURES) | \
> > >   (1ULL << VIRTIO_NET_F_HOST_TSO4) | \
> > >   (1ULL << VIRTIO_NET_F_HOST_TSO6) | \
> > > - (1ULL << VIRTIO_NET_F_CSUM))
> > > + (1ULL << VIRTIO_NET_F_CSUM)| \
> > > + (1ULL << VIRTIO_NET_F_GUEST_CSUM) | \
> > > + (1ULL << VIRTIO_NET_F_GUEST_TSO4) | \
> > > + (1ULL << VIRTIO_NET_F_GUEST_TSO6))
> > 
> > I don't think we need that, and it might be wrong to set those fields at 
> > vhost.
> > 
> > And, TBH, I am not 100% sure that I understand what those flags truely are
> > and for. All I know is that they seem have something to do with QEMU/TAP
> > device.
> 
> According to virtio standard, those fileds should be set.

If so, you'd better quote them here, or even to your patch commit.

--yliu

> Yes, I'd like to listen other guys comments.
> 
> > Hopefully the virtio expert, Michael, could shine some lights on that.
> > 
> > --yliu
> > 
> > >
> > >  static uint64_t VHOST_FEATURES = VHOST_SUPPORTED_FEATURES;
> > >
> > > --
> > > 1.7.7.6


[dpdk-dev] [PATCH v4 7/8] virtio/lib:add virtio guest offload capabilities

2015-11-11 Thread Yuanhan Liu
On Wed, Nov 11, 2015 at 02:40:45PM +0800, Jijiang Liu wrote:
> Add virtio guest offload capabilities.
> 
> Signed-off-by: Jijiang Liu 
> ---
>  lib/librte_vhost/virtio-net.c |5 -
>  1 files changed, 4 insertions(+), 1 deletions(-)
> 
> diff --git a/lib/librte_vhost/virtio-net.c b/lib/librte_vhost/virtio-net.c
> index 81bd309..839a333 100644
> --- a/lib/librte_vhost/virtio-net.c
> +++ b/lib/librte_vhost/virtio-net.c
> @@ -80,7 +80,10 @@ static struct virtio_net_config_ll *ll_root;
>   (1ULL << VHOST_USER_F_PROTOCOL_FEATURES) | \
>   (1ULL << VIRTIO_NET_F_HOST_TSO4) | \
>   (1ULL << VIRTIO_NET_F_HOST_TSO6) | \
> - (1ULL << VIRTIO_NET_F_CSUM))
> + (1ULL << VIRTIO_NET_F_CSUM)| \
> + (1ULL << VIRTIO_NET_F_GUEST_CSUM) | \
> + (1ULL << VIRTIO_NET_F_GUEST_TSO4) | \
> + (1ULL << VIRTIO_NET_F_GUEST_TSO6))

I don't think we need that, and it might be wrong to set those fields at
vhost.

And, TBH, I am not 100% sure that I understand what those flags truely are
and for. All I know is that they seem have something to do with QEMU/TAP
device.

Hopefully the virtio expert, Michael, could shine some lights on that.

--yliu

>  
>  static uint64_t VHOST_FEATURES = VHOST_SUPPORTED_FEATURES;
>  
> -- 
> 1.7.7.6


[dpdk-dev] [PATCH v4 7/8] virtio/lib:add virtio guest offload capabilities

2015-11-11 Thread Jijiang Liu
Add virtio guest offload capabilities.

Signed-off-by: Jijiang Liu 
---
 lib/librte_vhost/virtio-net.c |5 -
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/lib/librte_vhost/virtio-net.c b/lib/librte_vhost/virtio-net.c
index 81bd309..839a333 100644
--- a/lib/librte_vhost/virtio-net.c
+++ b/lib/librte_vhost/virtio-net.c
@@ -80,7 +80,10 @@ static struct virtio_net_config_ll *ll_root;
(1ULL << VHOST_USER_F_PROTOCOL_FEATURES) | \
(1ULL << VIRTIO_NET_F_HOST_TSO4) | \
(1ULL << VIRTIO_NET_F_HOST_TSO6) | \
-   (1ULL << VIRTIO_NET_F_CSUM))
+   (1ULL << VIRTIO_NET_F_CSUM)| \
+   (1ULL << VIRTIO_NET_F_GUEST_CSUM) | \
+   (1ULL << VIRTIO_NET_F_GUEST_TSO4) | \
+   (1ULL << VIRTIO_NET_F_GUEST_TSO6))

 static uint64_t VHOST_FEATURES = VHOST_SUPPORTED_FEATURES;

-- 
1.7.7.6



[dpdk-dev] [PATCH v4 7/8] virtio/lib:add virtio guest offload capabilities

2015-11-11 Thread Liu, Jijiang
Hi Qian,


> 
> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Liu, Jijiang
> Sent: Wednesday, November 11, 2015 4:53 PM
> To: Yuanhan Liu
> Cc: dev at dpdk.org; Michael S. Tsirkin
> Subject: Re: [dpdk-dev] [PATCH v4 7/8] virtio/lib:add virtio guest offload
> capabilities
> 
> 
> 
> > -Original Message-
> > From: Yuanhan Liu [mailto:yuanhan.liu at linux.intel.com]
> > Sent: Wednesday, November 11, 2015 4:44 PM
> > To: Liu, Jijiang
> > Cc: Michael S. Tsirkin; dev at dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH v4 7/8] virtio/lib:add virtio guest
> > offload capabilities
> >
> > On Wed, Nov 11, 2015 at 08:38:29AM +, Liu, Jijiang wrote:
> > >
> > >
> > > > -Original Message-
> > > > From: Yuanhan Liu [mailto:yuanhan.liu at linux.intel.com]
> > > > Sent: Wednesday, November 11, 2015 4:31 PM
> > > > To: Liu, Jijiang; Michael S. Tsirkin
> > > > Cc: dev at dpdk.org
> > > > Subject: Re: [dpdk-dev] [PATCH v4 7/8] virtio/lib:add virtio guest
> > > > offload capabilities
> > > >
> > > > I don't think we need that, and it might be wrong to set those
> > > > fields at
> > vhost.
> > > >
> > > > And, TBH, I am not 100% sure that I understand what those flags
> > > > truely are and for. All I know is that they seem have something to
> > > > do with QEMU/TAP device.
> > >
> > > According to virtio standard, those fileds should be set.
> >
> > If so, you'd better quote them here, or even to your patch commit.
> 
> In the Virtual I/O Device (VIRTIO) Version 1.0 Committee Specification 02,
> 
> 
> [QIAN] Do we support virtio 1.0 in the vhost offload case? Currently it's 
> virtio
> 0.95 Spec that we are testing.
> 
There are almost same description in  0.95 Spec,

(1)VIRTIO_NET_F_GUEST_CSUM (1) Guest handles packets with
partial checksum
(2) If the VIRTIO_NET_F_GUEST_CSUM feature was negotiated, the
VIRTIO_NET_HDR_F_NEEDS_CSUM bit in the ags eld may be
set: if so, the checksum on the packet is incomplete and the csum_start
and csum_oset elds indicate how to calculate it (see 1).

(3) The converse features are also available: a driver can save the virtual 
device
some work by negotiating these features.8 The VIRTIO_NET_F_GUEST_CSUM
feature indicates that partially checksummed packets can be received,
and if it can do that then the VIRTIO_NET_F_GUEST_TSO4, VIRTIO_
NET_F_GUEST_TSO6, VIRTIO_NET_F_GUEST_UFO and
VIRTIO_NET_F_GUEST_ECN are the input equivalents of the features
described above. See Receiving Packets below.



[dpdk-dev] [PATCH v4 7/8] virtio/lib:add virtio guest offload capabilities

2015-11-11 Thread Thomas Monjalon
2015-11-11 08:53, Liu, Jijiang:
> From: Yuanhan Liu [mailto:yuanhan.liu at linux.intel.com]
> > On Wed, Nov 11, 2015 at 08:38:29AM +, Liu, Jijiang wrote:
> > > According to virtio standard, those fileds should be set.
> > 
> > If so, you'd better quote them here, or even to your patch commit.
> 
> In the Virtual I/O Device (VIRTIO) Version 1.0 Committee Specification 02, 
> 
> (1) VIRTIO_NET_F_GUEST_CSUM (1) Driver handles packets with partial checksum.
> 
> (2) If the VIRTIO_NET_F_GUEST_CSUM feature was negotiated, the 
> VIRTIO_NET_HDR_F_NEEDS_-
> CSUM bit in flags MAY be set: if so, the checksum on the packet is incomplete 
> and csum_start
> and csum_offset indicate how to calculate it (see Packet Transmission point 
> 1).
> 
> (3) If the VIRTIO_NET_F_GUEST_TSO4, TSO6 or UFO options were negotiated, then 
> gso_type
> MAY be something other than VIRTIO_NET_HDR_GSO_NONE, and gso_size field 
> indicates
> the desired MSS (see Packet Transmission point 2).
> 
> The information can be included in the next version of patch set.

Yes quoting specifications or datasheets in commit messages is a really good 
idea.
Thanks for improving the quality of the git history.


[dpdk-dev] [PATCH v4 7/8] virtio/lib:add virtio guest offload capabilities

2015-11-11 Thread Liu, Jijiang


> -Original Message-
> From: Yuanhan Liu [mailto:yuanhan.liu at linux.intel.com]
> Sent: Wednesday, November 11, 2015 4:44 PM
> To: Liu, Jijiang
> Cc: Michael S. Tsirkin; dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v4 7/8] virtio/lib:add virtio guest offload
> capabilities
> 
> On Wed, Nov 11, 2015 at 08:38:29AM +, Liu, Jijiang wrote:
> >
> >
> > > -Original Message-
> > > From: Yuanhan Liu [mailto:yuanhan.liu at linux.intel.com]
> > > Sent: Wednesday, November 11, 2015 4:31 PM
> > > To: Liu, Jijiang; Michael S. Tsirkin
> > > Cc: dev at dpdk.org
> > > Subject: Re: [dpdk-dev] [PATCH v4 7/8] virtio/lib:add virtio guest
> > > offload capabilities
> > >
> > > On Wed, Nov 11, 2015 at 02:40:45PM +0800, Jijiang Liu wrote:
> > > > Add virtio guest offload capabilities.
> > > >
> > > > Signed-off-by: Jijiang Liu 
> > > > ---
> > > >  lib/librte_vhost/virtio-net.c |5 -
> > > >  1 files changed, 4 insertions(+), 1 deletions(-)
> > > >
> > > > diff --git a/lib/librte_vhost/virtio-net.c
> > > > b/lib/librte_vhost/virtio-net.c index 81bd309..839a333 100644
> > > > --- a/lib/librte_vhost/virtio-net.c
> > > > +++ b/lib/librte_vhost/virtio-net.c
> > > > @@ -80,7 +80,10 @@ static struct virtio_net_config_ll *ll_root;
> > > > (1ULL <<
> > > VHOST_USER_F_PROTOCOL_FEATURES) | \
> > > > (1ULL << VIRTIO_NET_F_HOST_TSO4) | \
> > > > (1ULL << VIRTIO_NET_F_HOST_TSO6) | \
> > > > -   (1ULL << VIRTIO_NET_F_CSUM))
> > > > +   (1ULL << VIRTIO_NET_F_CSUM)| \
> > > > +   (1ULL << VIRTIO_NET_F_GUEST_CSUM) | \
> > > > +   (1ULL << VIRTIO_NET_F_GUEST_TSO4) | \
> > > > +   (1ULL << VIRTIO_NET_F_GUEST_TSO6))
> > >
> > > I don't think we need that, and it might be wrong to set those fields at
> vhost.
> > >
> > > And, TBH, I am not 100% sure that I understand what those flags
> > > truely are and for. All I know is that they seem have something to
> > > do with QEMU/TAP device.
> >
> > According to virtio standard, those fileds should be set.
> 
> If so, you'd better quote them here, or even to your patch commit.

In the Virtual I/O Device (VIRTIO) Version 1.0 Committee Specification 02, 

(1) VIRTIO_NET_F_GUEST_CSUM (1) Driver handles packets with partial checksum.

(2) If the VIRTIO_NET_F_GUEST_CSUM feature was negotiated, the 
VIRTIO_NET_HDR_F_NEEDS_-
CSUM bit in flags MAY be set: if so, the checksum on the packet is incomplete 
and csum_start
and csum_offset indicate how to calculate it (see Packet Transmission point 1).

(3) If the VIRTIO_NET_F_GUEST_TSO4, TSO6 or UFO options were negotiated, then 
gso_type
MAY be something other than VIRTIO_NET_HDR_GSO_NONE, and gso_size field 
indicates
the desired MSS (see Packet Transmission point 2).

The information can be included in the next version of patch set.

>   --yliu
> 
> > Yes, I'd like to listen other guys comments.
> >
> > > Hopefully the virtio expert, Michael, could shine some lights on that.
> > >
> > >   --yliu
> > >
> > > >
> > > >  static uint64_t VHOST_FEATURES = VHOST_SUPPORTED_FEATURES;
> > > >
> > > > --
> > > > 1.7.7.6


[dpdk-dev] [PATCH v4 7/8] virtio/lib:add virtio guest offload capabilities

2015-11-11 Thread Liu, Jijiang


> -Original Message-
> From: Yuanhan Liu [mailto:yuanhan.liu at linux.intel.com]
> Sent: Wednesday, November 11, 2015 4:31 PM
> To: Liu, Jijiang; Michael S. Tsirkin
> Cc: dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v4 7/8] virtio/lib:add virtio guest offload
> capabilities
> 
> On Wed, Nov 11, 2015 at 02:40:45PM +0800, Jijiang Liu wrote:
> > Add virtio guest offload capabilities.
> >
> > Signed-off-by: Jijiang Liu 
> > ---
> >  lib/librte_vhost/virtio-net.c |5 -
> >  1 files changed, 4 insertions(+), 1 deletions(-)
> >
> > diff --git a/lib/librte_vhost/virtio-net.c
> > b/lib/librte_vhost/virtio-net.c index 81bd309..839a333 100644
> > --- a/lib/librte_vhost/virtio-net.c
> > +++ b/lib/librte_vhost/virtio-net.c
> > @@ -80,7 +80,10 @@ static struct virtio_net_config_ll *ll_root;
> > (1ULL <<
> VHOST_USER_F_PROTOCOL_FEATURES) | \
> > (1ULL << VIRTIO_NET_F_HOST_TSO4) | \
> > (1ULL << VIRTIO_NET_F_HOST_TSO6) | \
> > -   (1ULL << VIRTIO_NET_F_CSUM))
> > +   (1ULL << VIRTIO_NET_F_CSUM)| \
> > +   (1ULL << VIRTIO_NET_F_GUEST_CSUM) | \
> > +   (1ULL << VIRTIO_NET_F_GUEST_TSO4) | \
> > +   (1ULL << VIRTIO_NET_F_GUEST_TSO6))
> 
> I don't think we need that, and it might be wrong to set those fields at 
> vhost.
> 
> And, TBH, I am not 100% sure that I understand what those flags truely are
> and for. All I know is that they seem have something to do with QEMU/TAP
> device.

According to virtio standard, those fileds should be set.
Yes, I'd like to listen other guys comments.

> Hopefully the virtio expert, Michael, could shine some lights on that.
> 
>   --yliu
> 
> >
> >  static uint64_t VHOST_FEATURES = VHOST_SUPPORTED_FEATURES;
> >
> > --
> > 1.7.7.6