netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net V2 1/2] tun: unbreak truncated packet signalling
@ 2013-12-10  5:49 Jason Wang
  2013-12-10  5:49 ` [PATCH net V2 2/2] macvtap: signal truncated packets Jason Wang
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Jason Wang @ 2013-12-10  5:49 UTC (permalink / raw)
  To: davem, netdev, linux-kernel
  Cc: Jason Wang, Zhi Yong Wu, Michael S. Tsirkin, Vlad Yasevich

Commit 6680ec68eff47d36f67b4351bc9836fd6cba9532
(tuntap: hardware vlan tx support) breaks the truncated packet signal by nev
return a length greater than iov length in tun_put_user(). This patch fixes
by always return the length of packet plus possible vlan header. Caller can
detect the truncated packet by comparing the return value and the size of io
length.

Cc: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Vlad Yasevich <vyasevich@gmail.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
Changes from v1:
- increase total unconditionally
- do not move veth structure out of the vlan handing block
---
 drivers/net/tun.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index e26cbea..cd142134 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -1184,7 +1184,7 @@ static ssize_t tun_put_user(struct tun_struct *tun,
 {
 	struct tun_pi pi = { 0, skb->protocol };
 	ssize_t total = 0;
-	int vlan_offset = 0;
+	int vlan_offset = 0, offset;
 
 	if (!(tun->flags & TUN_NO_PI)) {
 		if ((len -= sizeof(pi)) < 0)
@@ -1248,6 +1248,8 @@ static ssize_t tun_put_user(struct tun_struct *tun,
 		total += tun->vnet_hdr_sz;
 	}
 
+	offset = total;
+	total += skb->len;
 	if (!vlan_tx_tag_present(skb)) {
 		len = min_t(int, skb->len, len);
 	} else {
@@ -1262,24 +1264,24 @@ static ssize_t tun_put_user(struct tun_struct *tun,
 
 		vlan_offset = offsetof(struct vlan_ethhdr, h_vlan_proto);
 		len = min_t(int, skb->len + VLAN_HLEN, len);
+		total += VLAN_HLEN;
 
 		copy = min_t(int, vlan_offset, len);
-		ret = skb_copy_datagram_const_iovec(skb, 0, iv, total, copy);
+		ret = skb_copy_datagram_const_iovec(skb, 0, iv, offset, copy);
 		len -= copy;
-		total += copy;
+		offset += copy;
 		if (ret || !len)
 			goto done;
 
 		copy = min_t(int, sizeof(veth), len);
-		ret = memcpy_toiovecend(iv, (void *)&veth, total, copy);
+		ret = memcpy_toiovecend(iv, (void *)&veth, offset, copy);
 		len -= copy;
-		total += copy;
+		offset += copy;
 		if (ret || !len)
 			goto done;
 	}
 
-	skb_copy_datagram_const_iovec(skb, vlan_offset, iv, total, len);
-	total += len;
+	skb_copy_datagram_const_iovec(skb, vlan_offset, iv, offset, len);
 
 done:
 	tun->dev->stats.tx_packets++;
-- 
1.8.3.2

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH net V2 2/2] macvtap: signal truncated packets
  2013-12-10  5:49 [PATCH net V2 1/2] tun: unbreak truncated packet signalling Jason Wang
@ 2013-12-10  5:49 ` Jason Wang
  2013-12-10 15:29   ` Michael S. Tsirkin
  2013-12-10 15:32 ` [PATCH net V2 1/2] tun: unbreak truncated packet signalling Michael S. Tsirkin
  2013-12-11  3:11 ` David Miller
  2 siblings, 1 reply; 8+ messages in thread
From: Jason Wang @ 2013-12-10  5:49 UTC (permalink / raw)
  To: davem, netdev, linux-kernel
  Cc: Jason Wang, Vlad Yasevich, Zhi Yong Wu, Michael S. Tsirkin

macvtap_put_user() never return a value grater than iov length, this in fact
bypasses the truncated checking in macvtap_recvmsg(). Fix this by always
returning the size of packet plus the possible vlan header to let the trunca
checking work.

Cc: Vlad Yasevich <vyasevich@gmail.com>
Cc: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
Changes from v1:
- increase total unconditionally
- do not move the structure veth out of the vlan handling block
---
 drivers/net/macvtap.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index 957cc5c..ded4b2c 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -770,7 +770,7 @@ static ssize_t macvtap_put_user(struct macvtap_queue *q,
 	int ret;
 	int vnet_hdr_len = 0;
 	int vlan_offset = 0;
-	int copied;
+	int copied, offset;
 
 	if (q->flags & IFF_VNET_HDR) {
 		struct virtio_net_hdr vnet_hdr;
@@ -785,7 +785,8 @@ static ssize_t macvtap_put_user(struct macvtap_queue *q,
 		if (memcpy_toiovecend(iv, (void *)&vnet_hdr, 0, sizeof(vnet_hdr)))
 			return -EFAULT;
 	}
-	copied = vnet_hdr_len;
+	offset = copied = vnet_hdr_len;
+	copied += skb->len;
 
 	if (!vlan_tx_tag_present(skb))
 		len = min_t(int, skb->len, len);
@@ -800,24 +801,24 @@ static ssize_t macvtap_put_user(struct macvtap_queue *q,
 
 		vlan_offset = offsetof(struct vlan_ethhdr, h_vlan_proto);
 		len = min_t(int, skb->len + VLAN_HLEN, len);
+		copied += VLAN_HLEN;
 
 		copy = min_t(int, vlan_offset, len);
-		ret = skb_copy_datagram_const_iovec(skb, 0, iv, copied, copy);
+		ret = skb_copy_datagram_const_iovec(skb, 0, iv, offset, copy);
 		len -= copy;
-		copied += copy;
+		offset += copy;
 		if (ret || !len)
 			goto done;
 
 		copy = min_t(int, sizeof(veth), len);
-		ret = memcpy_toiovecend(iv, (void *)&veth, copied, copy);
+		ret = memcpy_toiovecend(iv, (void *)&veth, offset, copy);
 		len -= copy;
-		copied += copy;
+		offset += copy;
 		if (ret || !len)
 			goto done;
 	}
 
-	ret = skb_copy_datagram_const_iovec(skb, vlan_offset, iv, copied, len);
-	copied += len;
+	ret = skb_copy_datagram_const_iovec(skb, vlan_offset, iv, offset, len);
 
 done:
 	return ret ? ret : copied;
@@ -875,7 +876,7 @@ static ssize_t macvtap_aio_read(struct kiocb *iocb, const struct iovec *iv,
 	}
 
 	ret = macvtap_do_read(q, iocb, iv, len, file->f_flags & O_NONBLOCK);
-	ret = min_t(ssize_t, ret, len); /* XXX copied from tun.c. Why? */
+	ret = min_t(ssize_t, ret, len);
 	if (ret > 0)
 		iocb->ki_pos = ret;
 out:
-- 
1.8.3.2

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH net V2 2/2] macvtap: signal truncated packets
  2013-12-10  5:49 ` [PATCH net V2 2/2] macvtap: signal truncated packets Jason Wang
@ 2013-12-10 15:29   ` Michael S. Tsirkin
  2013-12-11  3:29     ` Jason Wang
  0 siblings, 1 reply; 8+ messages in thread
From: Michael S. Tsirkin @ 2013-12-10 15:29 UTC (permalink / raw)
  To: Jason Wang; +Cc: davem, netdev, linux-kernel, Vlad Yasevich, Zhi Yong Wu

On Tue, Dec 10, 2013 at 01:49:46PM +0800, Jason Wang wrote:
> macvtap_put_user() never return a value grater than iov length, this in fact
> bypasses the truncated checking in macvtap_recvmsg(). Fix this by always
> returning the size of packet plus the possible vlan header to let the trunca
> checking work.
> 
> Cc: Vlad Yasevich <vyasevich@gmail.com>
> Cc: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
> Changes from v1:
> - increase total unconditionally
> - do not move the structure veth out of the vlan handling block
> ---
>  drivers/net/macvtap.c | 19 ++++++++++---------
>  1 file changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
> index 957cc5c..ded4b2c 100644
> --- a/drivers/net/macvtap.c
> +++ b/drivers/net/macvtap.c
> @@ -770,7 +770,7 @@ static ssize_t macvtap_put_user(struct macvtap_queue *q,
>  	int ret;
>  	int vnet_hdr_len = 0;
>  	int vlan_offset = 0;
> -	int copied;
> +	int copied, offset;
>  
>  	if (q->flags & IFF_VNET_HDR) {
>  		struct virtio_net_hdr vnet_hdr;
> @@ -785,7 +785,8 @@ static ssize_t macvtap_put_user(struct macvtap_queue *q,
>  		if (memcpy_toiovecend(iv, (void *)&vnet_hdr, 0, sizeof(vnet_hdr)))
>  			return -EFAULT;
>  	}
> -	copied = vnet_hdr_len;
> +	offset = copied = vnet_hdr_len;
> +	copied += skb->len;
>  
>  	if (!vlan_tx_tag_present(skb))
>  		len = min_t(int, skb->len, len);
> @@ -800,24 +801,24 @@ static ssize_t macvtap_put_user(struct macvtap_queue *q,
>  
>  		vlan_offset = offsetof(struct vlan_ethhdr, h_vlan_proto);
>  		len = min_t(int, skb->len + VLAN_HLEN, len);
> +		copied += VLAN_HLEN;
>  
>  		copy = min_t(int, vlan_offset, len);
> -		ret = skb_copy_datagram_const_iovec(skb, 0, iv, copied, copy);
> +		ret = skb_copy_datagram_const_iovec(skb, 0, iv, offset, copy);
>  		len -= copy;
> -		copied += copy;
> +		offset += copy;
>  		if (ret || !len)
>  			goto done;
>  
>  		copy = min_t(int, sizeof(veth), len);
> -		ret = memcpy_toiovecend(iv, (void *)&veth, copied, copy);
> +		ret = memcpy_toiovecend(iv, (void *)&veth, offset, copy);
>  		len -= copy;
> -		copied += copy;
> +		offset += copy;
>  		if (ret || !len)
>  			goto done;
>  	}
>  
> -	ret = skb_copy_datagram_const_iovec(skb, vlan_offset, iv, copied, len);
> -	copied += len;
> +	ret = skb_copy_datagram_const_iovec(skb, vlan_offset, iv, offset, len);
>  
>  done:
>  	return ret ? ret : copied;

I commented on this already. copied is how much we copied,
so its value is correct.
You want to name the new one total_len or something along
these lines and return it.

> @@ -875,7 +876,7 @@ static ssize_t macvtap_aio_read(struct kiocb *iocb, const struct iovec *iv,
>  	}
>  
>  	ret = macvtap_do_read(q, iocb, iv, len, file->f_flags & O_NONBLOCK);
> -	ret = min_t(ssize_t, ret, len); /* XXX copied from tun.c. Why? */
> +	ret = min_t(ssize_t, ret, len);
>  	if (ret > 0)
>  		iocb->ki_pos = ret;
>  out:
> -- 
> 1.8.3.2

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net V2 1/2] tun: unbreak truncated packet signalling
  2013-12-10  5:49 [PATCH net V2 1/2] tun: unbreak truncated packet signalling Jason Wang
  2013-12-10  5:49 ` [PATCH net V2 2/2] macvtap: signal truncated packets Jason Wang
@ 2013-12-10 15:32 ` Michael S. Tsirkin
  2013-12-11  3:28   ` Jason Wang
  2013-12-11  3:11 ` David Miller
  2 siblings, 1 reply; 8+ messages in thread
From: Michael S. Tsirkin @ 2013-12-10 15:32 UTC (permalink / raw)
  To: Jason Wang; +Cc: davem, netdev, linux-kernel, Zhi Yong Wu, Vlad Yasevich

On Tue, Dec 10, 2013 at 01:49:45PM +0800, Jason Wang wrote:
> Commit 6680ec68eff47d36f67b4351bc9836fd6cba9532
> (tuntap: hardware vlan tx support) breaks the truncated packet signal by nev
> return a length greater than iov length in tun_put_user(). This patch fixes
> by always return the length of packet plus possible vlan header. Caller can
> detect the truncated packet by comparing the return value and the size of io
> length.
> 
> Cc: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Vlad Yasevich <vyasevich@gmail.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
> Changes from v1:
> - increase total unconditionally
> - do not move veth structure out of the vlan handing block
> ---
>  drivers/net/tun.c | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index e26cbea..cd142134 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -1184,7 +1184,7 @@ static ssize_t tun_put_user(struct tun_struct *tun,
>  {
>  	struct tun_pi pi = { 0, skb->protocol };
>  	ssize_t total = 0;
> -	int vlan_offset = 0;
> +	int vlan_offset = 0, offset;
>  
>  	if (!(tun->flags & TUN_NO_PI)) {
>  		if ((len -= sizeof(pi)) < 0)
> @@ -1248,6 +1248,8 @@ static ssize_t tun_put_user(struct tun_struct *tun,
>  		total += tun->vnet_hdr_sz;
>  	}
>  
> +	offset = total;
> +	total += skb->len;
>  	if (!vlan_tx_tag_present(skb)) {
>  		len = min_t(int, skb->len, len);
>  	} else {
> @@ -1262,24 +1264,24 @@ static ssize_t tun_put_user(struct tun_struct *tun,
>  
>  		vlan_offset = offsetof(struct vlan_ethhdr, h_vlan_proto);
>  		len = min_t(int, skb->len + VLAN_HLEN, len);
> +		total += VLAN_HLEN;
>  
>  		copy = min_t(int, vlan_offset, len);
> -		ret = skb_copy_datagram_const_iovec(skb, 0, iv, total, copy);
> +		ret = skb_copy_datagram_const_iovec(skb, 0, iv, offset, copy);
>  		len -= copy;
> -		total += copy;
> +		offset += copy;
>  		if (ret || !len)
>  			goto done;
>  
>  		copy = min_t(int, sizeof(veth), len);
> -		ret = memcpy_toiovecend(iv, (void *)&veth, total, copy);
> +		ret = memcpy_toiovecend(iv, (void *)&veth, offset, copy);
>  		len -= copy;
> -		total += copy;
> +		offset += copy;
>  		if (ret || !len)
>  			goto done;
>  	}
>  
> -	skb_copy_datagram_const_iovec(skb, vlan_offset, iv, total, len);
> -	total += len;
> +	skb_copy_datagram_const_iovec(skb, vlan_offset, iv, offset, len);
>  
>  done:
>  	tun->dev->stats.tx_packets++;

offset is not descriptive. I would call new variable "copied",
and change all code to use that, do
total = copied + skb->len, total is then the total length.

> -- 
> 1.8.3.2

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net V2 1/2] tun: unbreak truncated packet signalling
  2013-12-10  5:49 [PATCH net V2 1/2] tun: unbreak truncated packet signalling Jason Wang
  2013-12-10  5:49 ` [PATCH net V2 2/2] macvtap: signal truncated packets Jason Wang
  2013-12-10 15:32 ` [PATCH net V2 1/2] tun: unbreak truncated packet signalling Michael S. Tsirkin
@ 2013-12-11  3:11 ` David Miller
  2013-12-11  3:27   ` Jason Wang
  2 siblings, 1 reply; 8+ messages in thread
From: David Miller @ 2013-12-11  3:11 UTC (permalink / raw)
  To: jasowang; +Cc: netdev, linux-kernel, wuzhy, mst, vyasevich


You and Michael are still discussing these changes it seems.

I accidently commited the first version of these patches, but then
immediately reverted that after I saw the followups.

Let me know when you have something both of you are happy with.

Thanks.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net V2 1/2] tun: unbreak truncated packet signalling
  2013-12-11  3:11 ` David Miller
@ 2013-12-11  3:27   ` Jason Wang
  0 siblings, 0 replies; 8+ messages in thread
From: Jason Wang @ 2013-12-11  3:27 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, linux-kernel, wuzhy, mst, vyasevich

On 12/11/2013 11:11 AM, David Miller wrote:
> You and Michael are still discussing these changes it seems.
>
> I accidently commited the first version of these patches, but then
> immediately reverted that after I saw the followups.
>
> Let me know when you have something both of you are happy with.
>
> Thanks.

Sure, I will post v3 which should address all comments from Michael.

Thanks

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net V2 1/2] tun: unbreak truncated packet signalling
  2013-12-10 15:32 ` [PATCH net V2 1/2] tun: unbreak truncated packet signalling Michael S. Tsirkin
@ 2013-12-11  3:28   ` Jason Wang
  0 siblings, 0 replies; 8+ messages in thread
From: Jason Wang @ 2013-12-11  3:28 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: davem, netdev, linux-kernel, Zhi Yong Wu, Vlad Yasevich

On 12/10/2013 11:32 PM, Michael S. Tsirkin wrote:
> On Tue, Dec 10, 2013 at 01:49:45PM +0800, Jason Wang wrote:
>> > Commit 6680ec68eff47d36f67b4351bc9836fd6cba9532
>> > (tuntap: hardware vlan tx support) breaks the truncated packet signal by nev
>> > return a length greater than iov length in tun_put_user(). This patch fixes
>> > by always return the length of packet plus possible vlan header. Caller can
>> > detect the truncated packet by comparing the return value and the size of io
>> > length.
>> > 
>> > Cc: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
>> > Cc: Michael S. Tsirkin <mst@redhat.com>
>> > Signed-off-by: Vlad Yasevich <vyasevich@gmail.com>
>> > Signed-off-by: Jason Wang <jasowang@redhat.com>
>> > ---
>> > Changes from v1:
>> > - increase total unconditionally
>> > - do not move veth structure out of the vlan handing block
>> > ---
>> >  drivers/net/tun.c | 16 +++++++++-------
>> >  1 file changed, 9 insertions(+), 7 deletions(-)
>> > 
>> > diff --git a/drivers/net/tun.c b/drivers/net/tun.c
>> > index e26cbea..cd142134 100644
>> > --- a/drivers/net/tun.c
>> > +++ b/drivers/net/tun.c
>> > @@ -1184,7 +1184,7 @@ static ssize_t tun_put_user(struct tun_struct *tun,
>> >  {
>> >  	struct tun_pi pi = { 0, skb->protocol };
>> >  	ssize_t total = 0;
>> > -	int vlan_offset = 0;
>> > +	int vlan_offset = 0, offset;
>> >  
>> >  	if (!(tun->flags & TUN_NO_PI)) {
>> >  		if ((len -= sizeof(pi)) < 0)
>> > @@ -1248,6 +1248,8 @@ static ssize_t tun_put_user(struct tun_struct *tun,
>> >  		total += tun->vnet_hdr_sz;
>> >  	}
>> >  
>> > +	offset = total;
>> > +	total += skb->len;
>> >  	if (!vlan_tx_tag_present(skb)) {
>> >  		len = min_t(int, skb->len, len);
>> >  	} else {
>> > @@ -1262,24 +1264,24 @@ static ssize_t tun_put_user(struct tun_struct *tun,
>> >  
>> >  		vlan_offset = offsetof(struct vlan_ethhdr, h_vlan_proto);
>> >  		len = min_t(int, skb->len + VLAN_HLEN, len);
>> > +		total += VLAN_HLEN;
>> >  
>> >  		copy = min_t(int, vlan_offset, len);
>> > -		ret = skb_copy_datagram_const_iovec(skb, 0, iv, total, copy);
>> > +		ret = skb_copy_datagram_const_iovec(skb, 0, iv, offset, copy);
>> >  		len -= copy;
>> > -		total += copy;
>> > +		offset += copy;
>> >  		if (ret || !len)
>> >  			goto done;
>> >  
>> >  		copy = min_t(int, sizeof(veth), len);
>> > -		ret = memcpy_toiovecend(iv, (void *)&veth, total, copy);
>> > +		ret = memcpy_toiovecend(iv, (void *)&veth, offset, copy);
>> >  		len -= copy;
>> > -		total += copy;
>> > +		offset += copy;
>> >  		if (ret || !len)
>> >  			goto done;
>> >  	}
>> >  
>> > -	skb_copy_datagram_const_iovec(skb, vlan_offset, iv, total, len);
>> > -	total += len;
>> > +	skb_copy_datagram_const_iovec(skb, vlan_offset, iv, offset, len);
>> >  
>> >  done:
>> >  	tun->dev->stats.tx_packets++;
> offset is not descriptive. I would call new variable "copied",
> and change all code to use that, do
> total = copied + skb->len, total is then the total length.
>

Ok, will do it in v3.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net V2 2/2] macvtap: signal truncated packets
  2013-12-10 15:29   ` Michael S. Tsirkin
@ 2013-12-11  3:29     ` Jason Wang
  0 siblings, 0 replies; 8+ messages in thread
From: Jason Wang @ 2013-12-11  3:29 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: davem, netdev, linux-kernel, Vlad Yasevich, Zhi Yong Wu

On 12/10/2013 11:29 PM, Michael S. Tsirkin wrote:
> On Tue, Dec 10, 2013 at 01:49:46PM +0800, Jason Wang wrote:
>> > macvtap_put_user() never return a value grater than iov length, this in fact
>> > bypasses the truncated checking in macvtap_recvmsg(). Fix this by always
>> > returning the size of packet plus the possible vlan header to let the trunca
>> > checking work.
>> > 
>> > Cc: Vlad Yasevich <vyasevich@gmail.com>
>> > Cc: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
>> > Cc: Michael S. Tsirkin <mst@redhat.com>
>> > Signed-off-by: Jason Wang <jasowang@redhat.com>
>> > ---
>> > Changes from v1:
>> > - increase total unconditionally
>> > - do not move the structure veth out of the vlan handling block
>> > ---
>> >  drivers/net/macvtap.c | 19 ++++++++++---------
>> >  1 file changed, 10 insertions(+), 9 deletions(-)
>> > 
>> > diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
>> > index 957cc5c..ded4b2c 100644
>> > --- a/drivers/net/macvtap.c
>> > +++ b/drivers/net/macvtap.c
>> > @@ -770,7 +770,7 @@ static ssize_t macvtap_put_user(struct macvtap_queue *q,
>> >  	int ret;
>> >  	int vnet_hdr_len = 0;
>> >  	int vlan_offset = 0;
>> > -	int copied;
>> > +	int copied, offset;
>> >  
>> >  	if (q->flags & IFF_VNET_HDR) {
>> >  		struct virtio_net_hdr vnet_hdr;
>> > @@ -785,7 +785,8 @@ static ssize_t macvtap_put_user(struct macvtap_queue *q,
>> >  		if (memcpy_toiovecend(iv, (void *)&vnet_hdr, 0, sizeof(vnet_hdr)))
>> >  			return -EFAULT;
>> >  	}
>> > -	copied = vnet_hdr_len;
>> > +	offset = copied = vnet_hdr_len;
>> > +	copied += skb->len;
>> >  
>> >  	if (!vlan_tx_tag_present(skb))
>> >  		len = min_t(int, skb->len, len);
>> > @@ -800,24 +801,24 @@ static ssize_t macvtap_put_user(struct macvtap_queue *q,
>> >  
>> >  		vlan_offset = offsetof(struct vlan_ethhdr, h_vlan_proto);
>> >  		len = min_t(int, skb->len + VLAN_HLEN, len);
>> > +		copied += VLAN_HLEN;
>> >  
>> >  		copy = min_t(int, vlan_offset, len);
>> > -		ret = skb_copy_datagram_const_iovec(skb, 0, iv, copied, copy);
>> > +		ret = skb_copy_datagram_const_iovec(skb, 0, iv, offset, copy);
>> >  		len -= copy;
>> > -		copied += copy;
>> > +		offset += copy;
>> >  		if (ret || !len)
>> >  			goto done;
>> >  
>> >  		copy = min_t(int, sizeof(veth), len);
>> > -		ret = memcpy_toiovecend(iv, (void *)&veth, copied, copy);
>> > +		ret = memcpy_toiovecend(iv, (void *)&veth, offset, copy);
>> >  		len -= copy;
>> > -		copied += copy;
>> > +		offset += copy;
>> >  		if (ret || !len)
>> >  			goto done;
>> >  	}
>> >  
>> > -	ret = skb_copy_datagram_const_iovec(skb, vlan_offset, iv, copied, len);
>> > -	copied += len;
>> > +	ret = skb_copy_datagram_const_iovec(skb, vlan_offset, iv, offset, len);
>> >  
>> >  done:
>> >  	return ret ? ret : copied;
> I commented on this already. copied is how much we copied,
> so its value is correct.
> You want to name the new one total_len or something along
> these lines and return it.
>

Ok, to be same with tun, I will use "total" in V3.

Thanks

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2013-12-11  3:29 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-10  5:49 [PATCH net V2 1/2] tun: unbreak truncated packet signalling Jason Wang
2013-12-10  5:49 ` [PATCH net V2 2/2] macvtap: signal truncated packets Jason Wang
2013-12-10 15:29   ` Michael S. Tsirkin
2013-12-11  3:29     ` Jason Wang
2013-12-10 15:32 ` [PATCH net V2 1/2] tun: unbreak truncated packet signalling Michael S. Tsirkin
2013-12-11  3:28   ` Jason Wang
2013-12-11  3:11 ` David Miller
2013-12-11  3:27   ` Jason Wang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).