netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next] skbuff: clear tx zero-copy flag
@ 2011-07-09  7:12 Shirley Ma
  2011-07-09  9:55 ` David Miller
  2011-07-25  0:42 ` Herbert Xu
  0 siblings, 2 replies; 9+ messages in thread
From: Shirley Ma @ 2011-07-09  7:12 UTC (permalink / raw)
  To: David Miller; +Cc: mst, netdev, kvm, linux-kernel

Hello Dave,

This patch clears tx zero-copy flag as needed.

Sign-off-by: Shirley Ma <xma@us.ibm.com>
---

 net/core/skbuff.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index a9577a2..d220119 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -677,6 +677,7 @@ struct sk_buff *skb_clone(struct sk_buff *skb, gfp_t gfp_mask)
 	if (skb_shinfo(skb)->tx_flags & SKBTX_DEV_ZEROCOPY) {
 		if (skb_copy_ubufs(skb, gfp_mask))
 			return NULL;
+		skb_shinfo(skb)->tx_flags &= ~SKBTX_DEV_ZEROCOPY;
 	}
 
 	n = skb + 1;
@@ -801,6 +802,7 @@ struct sk_buff *pskb_copy(struct sk_buff *skb, gfp_t gfp_mask)
 				kfree(n);
 				goto out;
 			}
+			skb_shinfo(skb)->tx_flags &= ~SKBTX_DEV_ZEROCOPY;
 		}
 		for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
 			skb_shinfo(n)->frags[i] = skb_shinfo(skb)->frags[i];
@@ -893,6 +895,7 @@ int pskb_expand_head(struct sk_buff *skb, int nhead, int ntail,
 		if (skb_shinfo(skb)->tx_flags & SKBTX_DEV_ZEROCOPY) {
 			if (skb_copy_ubufs(skb, gfp_mask))
 				goto nofrags;
+			skb_shinfo(skb)->tx_flags &= ~SKBTX_DEV_ZEROCOPY;
 		}
 		for (i = 0; i < skb_shinfo(skb)->nr_frags; i++)
 			get_page(skb_shinfo(skb)->frags[i].page);

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

* Re: [PATCH net-next] skbuff: clear tx zero-copy flag
  2011-07-09  7:12 [PATCH net-next] skbuff: clear tx zero-copy flag Shirley Ma
@ 2011-07-09  9:55 ` David Miller
  2011-07-25  0:42 ` Herbert Xu
  1 sibling, 0 replies; 9+ messages in thread
From: David Miller @ 2011-07-09  9:55 UTC (permalink / raw)
  To: mashirle; +Cc: mst, netdev, kvm, linux-kernel

From: Shirley Ma <mashirle@us.ibm.com>
Date: Sat, 09 Jul 2011 00:12:46 -0700

> This patch clears tx zero-copy flag as needed.
> 
> Signed-off-by: Shirley Ma <xma@us.ibm.com>

Applied, thanks.

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

* Re: [PATCH net-next] skbuff: clear tx zero-copy flag
  2011-07-09  7:12 [PATCH net-next] skbuff: clear tx zero-copy flag Shirley Ma
  2011-07-09  9:55 ` David Miller
@ 2011-07-25  0:42 ` Herbert Xu
  2011-07-25  8:07   ` Michael S. Tsirkin
  1 sibling, 1 reply; 9+ messages in thread
From: Herbert Xu @ 2011-07-25  0:42 UTC (permalink / raw)
  To: Shirley Ma; +Cc: davem, mst, netdev, kvm, linux-kernel

Shirley Ma <mashirle@us.ibm.com> wrote:
> 
> This patch clears tx zero-copy flag as needed.
> 
> Sign-off-by: Shirley Ma <xma@us.ibm.com>

I think we also need to copy and clear this flag on the splice
read path as that takes a direct page reference.

I hope there isn't any other path that does this.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

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

* Re: [PATCH net-next] skbuff: clear tx zero-copy flag
  2011-07-25  0:42 ` Herbert Xu
@ 2011-07-25  8:07   ` Michael S. Tsirkin
  2011-07-25  8:40     ` Herbert Xu
  0 siblings, 1 reply; 9+ messages in thread
From: Michael S. Tsirkin @ 2011-07-25  8:07 UTC (permalink / raw)
  To: Herbert Xu; +Cc: Shirley Ma, davem, netdev, kvm, linux-kernel

On Mon, Jul 25, 2011 at 08:42:00AM +0800, Herbert Xu wrote:
> Shirley Ma <mashirle@us.ibm.com> wrote:
> > 
> > This patch clears tx zero-copy flag as needed.
> > 
> > Sign-off-by: Shirley Ma <xma@us.ibm.com>
> 
> I think we also need to copy and clear this flag on the splice
> read path as that takes a direct page reference.
> 
> I hope there isn't any other path that does this.
> 
> Cheers,

When there's a way for an skb to get into the
host networking stack, (e.g. when tap gains zero copy
support) we'll need to handle that.

However macvtap passes an skb directly to the
lower device, so as long as macvtap is the only user
of that interface, we are fine I think - there's
no way for an skb to get from macvtap to splice
read path I think.

Right?

> -- 
> Email: Herbert Xu <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

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

* Re: [PATCH net-next] skbuff: clear tx zero-copy flag
  2011-07-25  8:07   ` Michael S. Tsirkin
@ 2011-07-25  8:40     ` Herbert Xu
  2011-07-25  9:44       ` Michael S. Tsirkin
  0 siblings, 1 reply; 9+ messages in thread
From: Herbert Xu @ 2011-07-25  8:40 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: Shirley Ma, davem, netdev, kvm, linux-kernel

On Mon, Jul 25, 2011 at 11:07:43AM +0300, Michael S. Tsirkin wrote:
>
> However macvtap passes an skb directly to the
> lower device, so as long as macvtap is the only user
> of that interface, we are fine I think - there's
> no way for an skb to get from macvtap to splice
> read path I think.
> 
> Right?

Yes, as long as you can guarantee that the skb never loops back
then you should be fine.

However, does macvtap really bypass everything, including the
qdisc layer? The qdisc layer is certainly capable of looping
the skb back with the redirect action.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

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

* Re: [PATCH net-next] skbuff: clear tx zero-copy flag
  2011-07-25  8:40     ` Herbert Xu
@ 2011-07-25  9:44       ` Michael S. Tsirkin
  2011-07-25  9:57         ` Herbert Xu
  0 siblings, 1 reply; 9+ messages in thread
From: Michael S. Tsirkin @ 2011-07-25  9:44 UTC (permalink / raw)
  To: Herbert Xu; +Cc: Shirley Ma, davem, netdev, kvm, linux-kernel

On Mon, Jul 25, 2011 at 04:40:57PM +0800, Herbert Xu wrote:
> On Mon, Jul 25, 2011 at 11:07:43AM +0300, Michael S. Tsirkin wrote:
> >
> > However macvtap passes an skb directly to the
> > lower device, so as long as macvtap is the only user
> > of that interface, we are fine I think - there's
> > no way for an skb to get from macvtap to splice
> > read path I think.
> > 
> > Right?
> 
> Yes, as long as you can guarantee that the skb never loops back
> then you should be fine.
> 
> However, does macvtap really bypass everything, including the
> qdisc layer? The qdisc layer is certainly capable of looping
> the skb back with the redirect action.
> 
> Cheers,

No, I don't think macvtap bypasses the qdisc.
Is the action in question here?
static int tcf_mirred(struct sk_buff *skb,
			const struct tc_action *a,
                      struct tcf_result *res)

if yes that seems to always clone an skb, which in turn
does the copy so we are fine?

> -- 
> Email: Herbert Xu <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

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

* Re: [PATCH net-next] skbuff: clear tx zero-copy flag
  2011-07-25  9:44       ` Michael S. Tsirkin
@ 2011-07-25  9:57         ` Herbert Xu
  2011-07-25 10:02           ` David Miller
  0 siblings, 1 reply; 9+ messages in thread
From: Herbert Xu @ 2011-07-25  9:57 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: Shirley Ma, davem, netdev, kvm, linux-kernel

On Mon, Jul 25, 2011 at 12:44:14PM +0300, Michael S. Tsirkin wrote:
>
> if yes that seems to always clone an skb, which in turn
> does the copy so we are fine?

Yes you're right, it should be safe.

However, I think we should add a WARN_ON to the splice skb path
so that should a packet find its way through a path that we haven't
thought of then at least we'll know about it.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

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

* Re: [PATCH net-next] skbuff: clear tx zero-copy flag
  2011-07-25  9:57         ` Herbert Xu
@ 2011-07-25 10:02           ` David Miller
  2011-07-25 10:53             ` Michael S. Tsirkin
  0 siblings, 1 reply; 9+ messages in thread
From: David Miller @ 2011-07-25 10:02 UTC (permalink / raw)
  To: herbert; +Cc: mst, mashirle, netdev, kvm, linux-kernel

From: Herbert Xu <herbert@gondor.hengli.com.au>
Date: Mon, 25 Jul 2011 17:57:11 +0800

> However, I think we should add a WARN_ON to the splice skb path
> so that should a packet find its way through a path that we haven't
> thought of then at least we'll know about it.

Good idea.

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

* Re: [PATCH net-next] skbuff: clear tx zero-copy flag
  2011-07-25 10:02           ` David Miller
@ 2011-07-25 10:53             ` Michael S. Tsirkin
  0 siblings, 0 replies; 9+ messages in thread
From: Michael S. Tsirkin @ 2011-07-25 10:53 UTC (permalink / raw)
  To: David Miller; +Cc: herbert, mashirle, netdev, kvm, linux-kernel

On Mon, Jul 25, 2011 at 03:02:29AM -0700, David Miller wrote:
> From: Herbert Xu <herbert@gondor.hengli.com.au>
> Date: Mon, 25 Jul 2011 17:57:11 +0800
> 
> > However, I think we should add a WARN_ON to the splice skb path
> > so that should a packet find its way through a path that we haven't
> > thought of then at least we'll know about it.
> 
> Good idea.

Another place like this is skb_split, I think.

-- 
MST

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

end of thread, other threads:[~2011-07-25 10:52 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-07-09  7:12 [PATCH net-next] skbuff: clear tx zero-copy flag Shirley Ma
2011-07-09  9:55 ` David Miller
2011-07-25  0:42 ` Herbert Xu
2011-07-25  8:07   ` Michael S. Tsirkin
2011-07-25  8:40     ` Herbert Xu
2011-07-25  9:44       ` Michael S. Tsirkin
2011-07-25  9:57         ` Herbert Xu
2011-07-25 10:02           ` David Miller
2011-07-25 10:53             ` Michael S. Tsirkin

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