netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH v3 00/12] Generic zcopy_* functions
@ 2020-12-30 19:12 Jonathan Lemon
  2020-12-30 19:12 ` [RFC PATCH v3 01/12] skbuff: remove unused skb_zcopy_abort function Jonathan Lemon
                   ` (12 more replies)
  0 siblings, 13 replies; 17+ messages in thread
From: Jonathan Lemon @ 2020-12-30 19:12 UTC (permalink / raw)
  To: netdev, willemdebruijn.kernel, edumazet, dsahern; +Cc: kernel-team

From: Jonathan Lemon <bsd@fb.com>

This is set of cleanup patches for zerocopy which are intended
to allow a introduction of a different zerocopy implementation.

The top level API will use the skb_zcopy_*() functions, while
the current TCP specific zerocopy ends up using msg_zerocopy_*()
calls.

There should be no functional changes from these patches.

v2->v3:
 Rename zc_flags to 'flags'.  Use SKBFL_xxx naming, similar
 to the SKBTX_xx naming.  Leave zerocopy_success naming alone.
 Reorder patches.

v1->v2:
 Break changes to skb_zcopy_put into 3 patches, in order to
 make it easier to follow the changes.  Add Willem's suggestion
 about renaming sock_zerocopy_

Patch 1: remove dead function
Patch 2: simplify sock_zerocopy_put
Patch 3: push status/refcounts into sock_zerocopy_callback
Patch 4: replace sock_zerocopy_put with skb_zcopy_put
Patch 5: rename sock_zerocopy_get
Patch 6:
  Add an optional skb parameter to callback, allowing access to
  the attached skb from the callback.
Patch 7:
  Add skb_zcopy_put_abort, and move zerocopy logic into the
  callback function.  There unfortunately is still a check
  against the callback type here.
Patch 8: Relocate skb_zcopy_clear() in skb_release_data()
Patch 9: rename sock_zerocopy_ to msg_zerocopy_
Patch 10:
  Move zerocopy bits from tx_flags into flags for clarity.
  These bits will be used in the RX path in the future.
Patch 11:
  Set the skb flags from the ubuf being attached, instead
  of a fixed value, allowing different initialization types.
Patch 12: Replace open-coded assignments with skb_zcopy_init()

Jonathan Lemon (12):
  skbuff: remove unused skb_zcopy_abort function
  skbuff: simplify sock_zerocopy_put
  skbuff: Push status and refcounts into sock_zerocopy_callback
  skbuff: replace sock_zerocopy_put() with skb_zcopy_put()
  skbuff: replace sock_zerocopy_get with skb_zcopy_get
  skbuff: Add skb parameter to the ubuf zerocopy callback
  skbuff: Call sock_zerocopy_put_abort from skb_zcopy_put_abort
  skbuff: Call skb_zcopy_clear() before unref'ing fragments
  skbuff: rename sock_zerocopy_* to msg_zerocopy_*
  net: group skb_shinfo zerocopy related bits together.
  skbuff: add flags to ubuf_info for ubuf setup
  tap/tun: add skb_zcopy_init() helper for initialization.

 drivers/net/tap.c                   |   6 +-
 drivers/net/tun.c                   |   6 +-
 drivers/net/xen-netback/common.h    |   3 +-
 drivers/net/xen-netback/interface.c |   4 +-
 drivers/net/xen-netback/netback.c   |   5 +-
 drivers/vhost/net.c                 |   4 +-
 include/linux/skbuff.h              | 106 +++++++++++++++-------------
 net/core/skbuff.c                   |  65 ++++++++---------
 net/ipv4/ip_output.c                |   5 +-
 net/ipv4/tcp.c                      |   8 +--
 net/ipv6/ip6_output.c               |   5 +-
 net/kcm/kcmsock.c                   |   4 +-
 12 files changed, 113 insertions(+), 108 deletions(-)

-- 
2.24.1


^ permalink raw reply	[flat|nested] 17+ messages in thread
* [RFC PATCH v3 01/12] skbuff: remove unused skb_zcopy_abort function
  2020-12-30 19:12 [RFC PATCH v3 00/12] Generic zcopy_* functions Jonathan Lemon
@ 2020-12-30 19:12 ` Jonathan Lemon
  2020-12-30 19:12 ` [RFC PATCH v3 02/12] skbuff: simplify sock_zerocopy_put Jonathan Lemon
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 17+ messages in thread
From: Jonathan Lemon @ 2020-12-30 19:12 UTC (permalink / raw)
  To: netdev, willemdebruijn.kernel, edumazet, dsahern; +Cc: kernel-team

From: Jonathan Lemon <bsd@fb.com>

skb_zcopy_abort() has no in-tree consumers, remove it.

Signed-off-by: Jonathan Lemon <jonathan.lemon@gmail.com>
Acked-by: Willem de Bruijn <willemb@google.com>
---
 include/linux/skbuff.h | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 333bcdc39635..3ca8d7c7b30c 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -1490,17 +1490,6 @@ static inline void skb_zcopy_clear(struct sk_buff *skb, bool zerocopy)
 	}
 }
 
-/* Abort a zerocopy operation and revert zckey on error in send syscall */
-static inline void skb_zcopy_abort(struct sk_buff *skb)
-{
-	struct ubuf_info *uarg = skb_zcopy(skb);
-
-	if (uarg) {
-		sock_zerocopy_put_abort(uarg, false);
-		skb_shinfo(skb)->tx_flags &= ~SKBTX_ZEROCOPY_FRAG;
-	}
-}
-
 static inline void skb_mark_not_on_list(struct sk_buff *skb)
 {
 	skb->next = NULL;
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread
* [RFC PATCH v3 02/12] skbuff: simplify sock_zerocopy_put
  2020-12-30 19:12 [RFC PATCH v3 00/12] Generic zcopy_* functions Jonathan Lemon
  2020-12-30 19:12 ` [RFC PATCH v3 01/12] skbuff: remove unused skb_zcopy_abort function Jonathan Lemon
@ 2020-12-30 19:12 ` Jonathan Lemon
  2020-12-30 19:12 ` [RFC PATCH v3 03/12] skbuff: Push status and refcounts into sock_zerocopy_callback Jonathan Lemon
                   ` (10 subsequent siblings)
  12 siblings, 0 replies; 17+ messages in thread
From: Jonathan Lemon @ 2020-12-30 19:12 UTC (permalink / raw)
  To: netdev, willemdebruijn.kernel, edumazet, dsahern; +Cc: kernel-team

From: Jonathan Lemon <bsd@fb.com>

All 'struct ubuf_info' users should have a callback defined
as of commit 0a4a060bb204 ("sock: fix zerocopy_success regression
with msg_zerocopy").

Remove the dead code path to consume_skb(), which makes
assumptions about how the structure was allocated.

Signed-off-by: Jonathan Lemon <jonathan.lemon@gmail.com>
Acked-by: Willem de Bruijn <willemb@google.com>
---
 net/core/skbuff.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index f62cae3f75d8..d88963f47f7d 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -1245,12 +1245,8 @@ EXPORT_SYMBOL_GPL(sock_zerocopy_callback);
 
 void sock_zerocopy_put(struct ubuf_info *uarg)
 {
-	if (uarg && refcount_dec_and_test(&uarg->refcnt)) {
-		if (uarg->callback)
-			uarg->callback(uarg, uarg->zerocopy);
-		else
-			consume_skb(skb_from_uarg(uarg));
-	}
+	if (uarg && refcount_dec_and_test(&uarg->refcnt))
+		uarg->callback(uarg, uarg->zerocopy);
 }
 EXPORT_SYMBOL_GPL(sock_zerocopy_put);
 
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread
* [RFC PATCH v3 03/12] skbuff: Push status and refcounts into sock_zerocopy_callback
  2020-12-30 19:12 [RFC PATCH v3 00/12] Generic zcopy_* functions Jonathan Lemon
  2020-12-30 19:12 ` [RFC PATCH v3 01/12] skbuff: remove unused skb_zcopy_abort function Jonathan Lemon
  2020-12-30 19:12 ` [RFC PATCH v3 02/12] skbuff: simplify sock_zerocopy_put Jonathan Lemon
@ 2020-12-30 19:12 ` Jonathan Lemon
  2020-12-30 19:12 ` [RFC PATCH v3 04/12] skbuff: replace sock_zerocopy_put() with skb_zcopy_put() Jonathan Lemon
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 17+ messages in thread
From: Jonathan Lemon @ 2020-12-30 19:12 UTC (permalink / raw)
  To: netdev, willemdebruijn.kernel, edumazet, dsahern; +Cc: kernel-team

From: Jonathan Lemon <bsd@fb.com>

Before this change, the caller of sock_zerocopy_callback would
need to save the zerocopy status, decrement and check the refcount,
and then call the callback function - the callback was only invoked
when the refcount reached zero.

Now, the caller just passes the status into the callback function,
which saves the status and handles its own refcounts.

This makes the behavior of the sock_zerocopy_callback identical
to the tpacket and vhost callbacks.

Signed-off-by: Jonathan Lemon <jonathan.lemon@gmail.com>
---
 include/linux/skbuff.h |  3 ---
 net/core/skbuff.c      | 14 +++++++++++---
 2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 3ca8d7c7b30c..52e96c35f5af 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -1479,9 +1479,6 @@ static inline void skb_zcopy_clear(struct sk_buff *skb, bool zerocopy)
 	if (uarg) {
 		if (skb_zcopy_is_nouarg(skb)) {
 			/* no notification callback */
-		} else if (uarg->callback == sock_zerocopy_callback) {
-			uarg->zerocopy = uarg->zerocopy && zerocopy;
-			sock_zerocopy_put(uarg);
 		} else {
 			uarg->callback(uarg, zerocopy);
 		}
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index d88963f47f7d..8c18940723ff 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -1194,7 +1194,7 @@ static bool skb_zerocopy_notify_extend(struct sk_buff *skb, u32 lo, u16 len)
 	return true;
 }
 
-void sock_zerocopy_callback(struct ubuf_info *uarg, bool success)
+static void __sock_zerocopy_callback(struct ubuf_info *uarg)
 {
 	struct sk_buff *tail, *skb = skb_from_uarg(uarg);
 	struct sock_exterr_skb *serr;
@@ -1222,7 +1222,7 @@ void sock_zerocopy_callback(struct ubuf_info *uarg, bool success)
 	serr->ee.ee_origin = SO_EE_ORIGIN_ZEROCOPY;
 	serr->ee.ee_data = hi;
 	serr->ee.ee_info = lo;
-	if (!success)
+	if (!uarg->zerocopy)
 		serr->ee.ee_code |= SO_EE_CODE_ZEROCOPY_COPIED;
 
 	q = &sk->sk_error_queue;
@@ -1241,11 +1241,19 @@ void sock_zerocopy_callback(struct ubuf_info *uarg, bool success)
 	consume_skb(skb);
 	sock_put(sk);
 }
+
+void sock_zerocopy_callback(struct ubuf_info *uarg, bool success)
+{
+	uarg->zerocopy = uarg->zerocopy & success;
+
+	if (refcount_dec_and_test(&uarg->refcnt))
+		__sock_zerocopy_callback(uarg);
+}
 EXPORT_SYMBOL_GPL(sock_zerocopy_callback);
 
 void sock_zerocopy_put(struct ubuf_info *uarg)
 {
-	if (uarg && refcount_dec_and_test(&uarg->refcnt))
+	if (uarg)
 		uarg->callback(uarg, uarg->zerocopy);
 }
 EXPORT_SYMBOL_GPL(sock_zerocopy_put);
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread
* [RFC PATCH v3 04/12] skbuff: replace sock_zerocopy_put() with skb_zcopy_put()
  2020-12-30 19:12 [RFC PATCH v3 00/12] Generic zcopy_* functions Jonathan Lemon
                   ` (2 preceding siblings ...)
  2020-12-30 19:12 ` [RFC PATCH v3 03/12] skbuff: Push status and refcounts into sock_zerocopy_callback Jonathan Lemon
@ 2020-12-30 19:12 ` Jonathan Lemon
  2020-12-30 19:12 ` [RFC PATCH v3 05/12] skbuff: replace sock_zerocopy_get with skb_zcopy_get Jonathan Lemon
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 17+ messages in thread
From: Jonathan Lemon @ 2020-12-30 19:12 UTC (permalink / raw)
  To: netdev, willemdebruijn.kernel, edumazet, dsahern; +Cc: kernel-team

From: Jonathan Lemon <bsd@fb.com>

Replace sock_zerocopy_put with the generic skb_zcopy_put()
function.  Pass 'true' as the success argument, as this
is identical to no change.

Signed-off-by: Jonathan Lemon <jonathan.lemon@gmail.com>
---
 include/linux/skbuff.h | 7 ++++++-
 net/core/skbuff.c      | 9 +--------
 net/ipv4/tcp.c         | 2 +-
 3 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 52e96c35f5af..a6c86839035b 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -496,7 +496,6 @@ static inline void sock_zerocopy_get(struct ubuf_info *uarg)
 	refcount_inc(&uarg->refcnt);
 }
 
-void sock_zerocopy_put(struct ubuf_info *uarg);
 void sock_zerocopy_put_abort(struct ubuf_info *uarg, bool have_uref);
 
 void sock_zerocopy_callback(struct ubuf_info *uarg, bool success);
@@ -1471,6 +1470,12 @@ static inline void *skb_zcopy_get_nouarg(struct sk_buff *skb)
 	return (void *)((uintptr_t) skb_shinfo(skb)->destructor_arg & ~0x1UL);
 }
 
+static inline void skb_zcopy_put(struct ubuf_info *uarg)
+{
+	if (uarg)
+		uarg->callback(uarg, true);
+}
+
 /* Release a reference on a zerocopy structure */
 static inline void skb_zcopy_clear(struct sk_buff *skb, bool zerocopy)
 {
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 8c18940723ff..0e028825367a 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -1251,13 +1251,6 @@ void sock_zerocopy_callback(struct ubuf_info *uarg, bool success)
 }
 EXPORT_SYMBOL_GPL(sock_zerocopy_callback);
 
-void sock_zerocopy_put(struct ubuf_info *uarg)
-{
-	if (uarg)
-		uarg->callback(uarg, uarg->zerocopy);
-}
-EXPORT_SYMBOL_GPL(sock_zerocopy_put);
-
 void sock_zerocopy_put_abort(struct ubuf_info *uarg, bool have_uref)
 {
 	if (uarg) {
@@ -1267,7 +1260,7 @@ void sock_zerocopy_put_abort(struct ubuf_info *uarg, bool have_uref)
 		uarg->len--;
 
 		if (have_uref)
-			sock_zerocopy_put(uarg);
+			skb_zcopy_put(uarg);
 	}
 }
 EXPORT_SYMBOL_GPL(sock_zerocopy_put_abort);
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index ed42d2193c5c..298a1fae841c 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -1429,7 +1429,7 @@ int tcp_sendmsg_locked(struct sock *sk, struct msghdr *msg, size_t size)
 		tcp_push(sk, flags, mss_now, tp->nonagle, size_goal);
 	}
 out_nopush:
-	sock_zerocopy_put(uarg);
+	skb_zcopy_put(uarg);
 	return copied + copied_syn;
 
 do_error:
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread
* [RFC PATCH v3 05/12] skbuff: replace sock_zerocopy_get with skb_zcopy_get
  2020-12-30 19:12 [RFC PATCH v3 00/12] Generic zcopy_* functions Jonathan Lemon
                   ` (3 preceding siblings ...)
  2020-12-30 19:12 ` [RFC PATCH v3 04/12] skbuff: replace sock_zerocopy_put() with skb_zcopy_put() Jonathan Lemon
@ 2020-12-30 19:12 ` Jonathan Lemon
  2020-12-30 19:12 ` [RFC PATCH v3 06/12] skbuff: Add skb parameter to the ubuf zerocopy callback Jonathan Lemon
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 17+ messages in thread
From: Jonathan Lemon @ 2020-12-30 19:12 UTC (permalink / raw)
  To: netdev, willemdebruijn.kernel, edumazet, dsahern; +Cc: kernel-team

From: Jonathan Lemon <bsd@fb.com>

Rename the get routines for consistency.

Signed-off-by: Jonathan Lemon <jonathan.lemon@gmail.com>
---
 include/linux/skbuff.h | 12 ++++++------
 net/core/skbuff.c      |  2 +-
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index a6c86839035b..5b8a53ab51fd 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -491,11 +491,6 @@ struct ubuf_info *sock_zerocopy_alloc(struct sock *sk, size_t size);
 struct ubuf_info *sock_zerocopy_realloc(struct sock *sk, size_t size,
 					struct ubuf_info *uarg);
 
-static inline void sock_zerocopy_get(struct ubuf_info *uarg)
-{
-	refcount_inc(&uarg->refcnt);
-}
-
 void sock_zerocopy_put_abort(struct ubuf_info *uarg, bool have_uref);
 
 void sock_zerocopy_callback(struct ubuf_info *uarg, bool success);
@@ -1441,6 +1436,11 @@ static inline struct ubuf_info *skb_zcopy(struct sk_buff *skb)
 	return is_zcopy ? skb_uarg(skb) : NULL;
 }
 
+static inline void skb_zcopy_get(struct ubuf_info *uarg)
+{
+	refcount_inc(&uarg->refcnt);
+}
+
 static inline void skb_zcopy_set(struct sk_buff *skb, struct ubuf_info *uarg,
 				 bool *have_ref)
 {
@@ -1448,7 +1448,7 @@ static inline void skb_zcopy_set(struct sk_buff *skb, struct ubuf_info *uarg,
 		if (unlikely(have_ref && *have_ref))
 			*have_ref = false;
 		else
-			sock_zerocopy_get(uarg);
+			skb_zcopy_get(uarg);
 		skb_shinfo(skb)->destructor_arg = uarg;
 		skb_shinfo(skb)->tx_flags |= SKBTX_ZEROCOPY_FRAG;
 	}
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 0e028825367a..00f195908e79 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -1163,7 +1163,7 @@ struct ubuf_info *sock_zerocopy_realloc(struct sock *sk, size_t size,
 
 			/* no extra ref when appending to datagram (MSG_MORE) */
 			if (sk->sk_type == SOCK_STREAM)
-				sock_zerocopy_get(uarg);
+				skb_zcopy_get(uarg);
 
 			return uarg;
 		}
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread
* [RFC PATCH v3 06/12] skbuff: Add skb parameter to the ubuf zerocopy callback
  2020-12-30 19:12 [RFC PATCH v3 00/12] Generic zcopy_* functions Jonathan Lemon
                   ` (4 preceding siblings ...)
  2020-12-30 19:12 ` [RFC PATCH v3 05/12] skbuff: replace sock_zerocopy_get with skb_zcopy_get Jonathan Lemon
@ 2020-12-30 19:12 ` Jonathan Lemon
  2020-12-30 19:12 ` [RFC PATCH v3 07/12] skbuff: Call sock_zerocopy_put_abort from skb_zcopy_put_abort Jonathan Lemon
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 17+ messages in thread
From: Jonathan Lemon @ 2020-12-30 19:12 UTC (permalink / raw)
  To: netdev, willemdebruijn.kernel, edumazet, dsahern; +Cc: kernel-team

From: Jonathan Lemon <bsd@fb.com>

Add an optional skb parameter to the zerocopy callback parameter,
which is passed down from skb_zcopy_clear().  This gives access
to the original skb, which is needed for upcoming RX zero-copy
error handling.

Signed-off-by: Jonathan Lemon <jonathan.lemon@gmail.com>
---
 drivers/net/tap.c                 |  2 +-
 drivers/net/tun.c                 |  2 +-
 drivers/net/xen-netback/common.h  |  3 ++-
 drivers/net/xen-netback/netback.c |  5 +++--
 drivers/vhost/net.c               |  3 ++-
 include/linux/skbuff.h            | 17 ++++++++---------
 net/core/skbuff.c                 |  3 ++-
 7 files changed, 19 insertions(+), 16 deletions(-)

diff --git a/drivers/net/tap.c b/drivers/net/tap.c
index 1f4bdd94407a..3f51f3766d18 100644
--- a/drivers/net/tap.c
+++ b/drivers/net/tap.c
@@ -727,7 +727,7 @@ static ssize_t tap_get_user(struct tap_queue *q, void *msg_control,
 		skb_shinfo(skb)->tx_flags |= SKBTX_SHARED_FRAG;
 	} else if (msg_control) {
 		struct ubuf_info *uarg = msg_control;
-		uarg->callback(uarg, false);
+		uarg->callback(NULL, uarg, false);
 	}
 
 	if (tap) {
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index fbed05ae7b0f..b64e2cc85751 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -1819,7 +1819,7 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
 		skb_shinfo(skb)->tx_flags |= SKBTX_SHARED_FRAG;
 	} else if (msg_control) {
 		struct ubuf_info *uarg = msg_control;
-		uarg->callback(uarg, false);
+		uarg->callback(NULL, uarg, false);
 	}
 
 	skb_reset_network_header(skb);
diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-netback/common.h
index 8ee24e351bdc..4a16d6e33c09 100644
--- a/drivers/net/xen-netback/common.h
+++ b/drivers/net/xen-netback/common.h
@@ -399,7 +399,8 @@ void xenvif_rx_queue_tail(struct xenvif_queue *queue, struct sk_buff *skb);
 void xenvif_carrier_on(struct xenvif *vif);
 
 /* Callback from stack when TX packet can be released */
-void xenvif_zerocopy_callback(struct ubuf_info *ubuf, bool zerocopy_success);
+void xenvif_zerocopy_callback(struct sk_buff *skb, struct ubuf_info *ubuf,
+			      bool zerocopy_success);
 
 /* Unmap a pending page and release it back to the guest */
 void xenvif_idx_unmap(struct xenvif_queue *queue, u16 pending_idx);
diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
index bc3421d14576..19a27dce79d2 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -1091,7 +1091,7 @@ static int xenvif_handle_frag_list(struct xenvif_queue *queue, struct sk_buff *s
 	uarg = skb_shinfo(skb)->destructor_arg;
 	/* increase inflight counter to offset decrement in callback */
 	atomic_inc(&queue->inflight_packets);
-	uarg->callback(uarg, true);
+	uarg->callback(NULL, uarg, true);
 	skb_shinfo(skb)->destructor_arg = NULL;
 
 	/* Fill the skb with the new (local) frags. */
@@ -1228,7 +1228,8 @@ static int xenvif_tx_submit(struct xenvif_queue *queue)
 	return work_done;
 }
 
-void xenvif_zerocopy_callback(struct ubuf_info *ubuf, bool zerocopy_success)
+void xenvif_zerocopy_callback(struct sk_buff *skb, struct ubuf_info *ubuf,
+			      bool zerocopy_success)
 {
 	unsigned long flags;
 	pending_ring_idx_t index;
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 531a00d703cd..bf28d0b75c1b 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -381,7 +381,8 @@ static void vhost_zerocopy_signal_used(struct vhost_net *net,
 	}
 }
 
-static void vhost_zerocopy_callback(struct ubuf_info *ubuf, bool success)
+static void vhost_zerocopy_callback(struct sk_buff *skb,
+				    struct ubuf_info *ubuf, bool success)
 {
 	struct vhost_net_ubuf_ref *ubufs = ubuf->ctx;
 	struct vhost_virtqueue *vq = ubufs->vq;
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 5b8a53ab51fd..b23c3b4b3209 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -461,7 +461,8 @@ enum {
  * The desc field is used to track userspace buffer index.
  */
 struct ubuf_info {
-	void (*callback)(struct ubuf_info *, bool zerocopy_success);
+	void (*callback)(struct sk_buff *, struct ubuf_info *,
+			 bool zerocopy_success);
 	union {
 		struct {
 			unsigned long desc;
@@ -493,7 +494,8 @@ struct ubuf_info *sock_zerocopy_realloc(struct sock *sk, size_t size,
 
 void sock_zerocopy_put_abort(struct ubuf_info *uarg, bool have_uref);
 
-void sock_zerocopy_callback(struct ubuf_info *uarg, bool success);
+void sock_zerocopy_callback(struct sk_buff *skb, struct ubuf_info *uarg,
+			    bool success);
 
 int skb_zerocopy_iter_dgram(struct sk_buff *skb, struct msghdr *msg, int len);
 int skb_zerocopy_iter_stream(struct sock *sk, struct sk_buff *skb,
@@ -1473,20 +1475,17 @@ static inline void *skb_zcopy_get_nouarg(struct sk_buff *skb)
 static inline void skb_zcopy_put(struct ubuf_info *uarg)
 {
 	if (uarg)
-		uarg->callback(uarg, true);
+		uarg->callback(NULL, uarg, true);
 }
 
 /* Release a reference on a zerocopy structure */
-static inline void skb_zcopy_clear(struct sk_buff *skb, bool zerocopy)
+static inline void skb_zcopy_clear(struct sk_buff *skb, bool zerocopy_success)
 {
 	struct ubuf_info *uarg = skb_zcopy(skb);
 
 	if (uarg) {
-		if (skb_zcopy_is_nouarg(skb)) {
-			/* no notification callback */
-		} else {
-			uarg->callback(uarg, zerocopy);
-		}
+		if (!skb_zcopy_is_nouarg(skb))
+			uarg->callback(skb, uarg, zerocopy_success);
 
 		skb_shinfo(skb)->tx_flags &= ~SKBTX_ZEROCOPY_FRAG;
 	}
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 00f195908e79..89130b21d9f0 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -1242,7 +1242,8 @@ static void __sock_zerocopy_callback(struct ubuf_info *uarg)
 	sock_put(sk);
 }
 
-void sock_zerocopy_callback(struct ubuf_info *uarg, bool success)
+void sock_zerocopy_callback(struct sk_buff *skb, struct ubuf_info *uarg,
+			    bool success)
 {
 	uarg->zerocopy = uarg->zerocopy & success;
 
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread
* [RFC PATCH v3 07/12] skbuff: Call sock_zerocopy_put_abort from skb_zcopy_put_abort
  2020-12-30 19:12 [RFC PATCH v3 00/12] Generic zcopy_* functions Jonathan Lemon
                   ` (5 preceding siblings ...)
  2020-12-30 19:12 ` [RFC PATCH v3 06/12] skbuff: Add skb parameter to the ubuf zerocopy callback Jonathan Lemon
@ 2020-12-30 19:12 ` Jonathan Lemon
  2020-12-30 19:12 ` [RFC PATCH v3 08/12] skbuff: Call skb_zcopy_clear() before unref'ing fragments Jonathan Lemon
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 17+ messages in thread
From: Jonathan Lemon @ 2020-12-30 19:12 UTC (permalink / raw)
  To: netdev, willemdebruijn.kernel, edumazet, dsahern; +Cc: kernel-team

From: Jonathan Lemon <bsd@fb.com>

The sock_zerocopy_put_abort function contains logic which is
specific to the current zerocopy implementation.  Add a wrapper
which checks the callback and dispatches apppropriately.

Signed-off-by: Jonathan Lemon <jonathan.lemon@gmail.com>
---
 include/linux/skbuff.h | 10 ++++++++++
 net/core/skbuff.c      | 12 +++++-------
 net/ipv4/ip_output.c   |  3 +--
 net/ipv4/tcp.c         |  2 +-
 net/ipv6/ip6_output.c  |  3 +--
 5 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index b23c3b4b3209..9f7393167f0a 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -1478,6 +1478,16 @@ static inline void skb_zcopy_put(struct ubuf_info *uarg)
 		uarg->callback(NULL, uarg, true);
 }
 
+static inline void skb_zcopy_put_abort(struct ubuf_info *uarg, bool have_uref)
+{
+	if (uarg) {
+		if (uarg->callback == sock_zerocopy_callback)
+			sock_zerocopy_put_abort(uarg, have_uref);
+		else if (have_uref)
+			skb_zcopy_put(uarg);
+	}
+}
+
 /* Release a reference on a zerocopy structure */
 static inline void skb_zcopy_clear(struct sk_buff *skb, bool zerocopy_success)
 {
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 89130b21d9f0..5b9cd528d6a6 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -1254,15 +1254,13 @@ EXPORT_SYMBOL_GPL(sock_zerocopy_callback);
 
 void sock_zerocopy_put_abort(struct ubuf_info *uarg, bool have_uref)
 {
-	if (uarg) {
-		struct sock *sk = skb_from_uarg(uarg)->sk;
+	struct sock *sk = skb_from_uarg(uarg)->sk;
 
-		atomic_dec(&sk->sk_zckey);
-		uarg->len--;
+	atomic_dec(&sk->sk_zckey);
+	uarg->len--;
 
-		if (have_uref)
-			skb_zcopy_put(uarg);
-	}
+	if (have_uref)
+		sock_zerocopy_callback(NULL, uarg, true);
 }
 EXPORT_SYMBOL_GPL(sock_zerocopy_put_abort);
 
diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index 879b76ae4435..65f2299fd682 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -1230,8 +1230,7 @@ static int __ip_append_data(struct sock *sk,
 error_efault:
 	err = -EFAULT;
 error:
-	if (uarg)
-		sock_zerocopy_put_abort(uarg, extra_uref);
+	skb_zcopy_put_abort(uarg, extra_uref);
 	cork->length -= length;
 	IP_INC_STATS(sock_net(sk), IPSTATS_MIB_OUTDISCARDS);
 	refcount_add(wmem_alloc_delta, &sk->sk_wmem_alloc);
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 298a1fae841c..fb58215972ba 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -1440,7 +1440,7 @@ int tcp_sendmsg_locked(struct sock *sk, struct msghdr *msg, size_t size)
 	if (copied + copied_syn)
 		goto out;
 out_err:
-	sock_zerocopy_put_abort(uarg, true);
+	skb_zcopy_put_abort(uarg, true);
 	err = sk_stream_error(sk, flags, err);
 	/* make sure we wake any epoll edge trigger waiter */
 	if (unlikely(tcp_rtx_and_write_queues_empty(sk) && err == -EAGAIN)) {
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 749ad72386b2..c8c87891533a 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -1715,8 +1715,7 @@ static int __ip6_append_data(struct sock *sk,
 error_efault:
 	err = -EFAULT;
 error:
-	if (uarg)
-		sock_zerocopy_put_abort(uarg, extra_uref);
+	skb_zcopy_put_abort(uarg, extra_uref);
 	cork->length -= length;
 	IP6_INC_STATS(sock_net(sk), rt->rt6i_idev, IPSTATS_MIB_OUTDISCARDS);
 	refcount_add(wmem_alloc_delta, &sk->sk_wmem_alloc);
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread
* [RFC PATCH v3 08/12] skbuff: Call skb_zcopy_clear() before unref'ing fragments
  2020-12-30 19:12 [RFC PATCH v3 00/12] Generic zcopy_* functions Jonathan Lemon
                   ` (6 preceding siblings ...)
  2020-12-30 19:12 ` [RFC PATCH v3 07/12] skbuff: Call sock_zerocopy_put_abort from skb_zcopy_put_abort Jonathan Lemon
@ 2020-12-30 19:12 ` Jonathan Lemon
  2020-12-30 19:12 ` [RFC PATCH v3 09/12] skbuff: rename sock_zerocopy_* to msg_zerocopy_* Jonathan Lemon
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 17+ messages in thread
From: Jonathan Lemon @ 2020-12-30 19:12 UTC (permalink / raw)
  To: netdev, willemdebruijn.kernel, edumazet, dsahern; +Cc: kernel-team

From: Jonathan Lemon <bsd@fb.com>

RX zerocopy fragment pages which are not allocated from the
system page pool require special handling.  Give the callback
in skb_zcopy_clear() a chance to process them first.

Signed-off-by: Jonathan Lemon <jonathan.lemon@gmail.com>
---
 net/core/skbuff.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 5b9cd528d6a6..6d031ed99182 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -605,13 +605,14 @@ static void skb_release_data(struct sk_buff *skb)
 			      &shinfo->dataref))
 		return;
 
+	skb_zcopy_clear(skb, true);
+
 	for (i = 0; i < shinfo->nr_frags; i++)
 		__skb_frag_unref(&shinfo->frags[i]);
 
 	if (shinfo->frag_list)
 		kfree_skb_list(shinfo->frag_list);
 
-	skb_zcopy_clear(skb, true);
 	skb_free_head(skb);
 }
 
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread
* [RFC PATCH v3 09/12] skbuff: rename sock_zerocopy_* to msg_zerocopy_*
  2020-12-30 19:12 [RFC PATCH v3 00/12] Generic zcopy_* functions Jonathan Lemon
                   ` (7 preceding siblings ...)
  2020-12-30 19:12 ` [RFC PATCH v3 08/12] skbuff: Call skb_zcopy_clear() before unref'ing fragments Jonathan Lemon
@ 2020-12-30 19:12 ` Jonathan Lemon
  2020-12-30 19:12 ` [RFC PATCH v3 10/12] net: group skb_shinfo zerocopy related bits together Jonathan Lemon
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 17+ messages in thread
From: Jonathan Lemon @ 2020-12-30 19:12 UTC (permalink / raw)
  To: netdev, willemdebruijn.kernel, edumazet, dsahern; +Cc: kernel-team

From: Jonathan Lemon <bsd@fb.com>

At Willem's suggestion, rename the sock_zerocopy_* functions
so that they match the MSG_ZEROCOPY flag, which makes it clear
they are specific to this zerocopy implementation.

Signed-off-by: Jonathan Lemon <jonathan.lemon@gmail.com>
Acked-by: Willem de Bruijn <willemb@google.com>
---
 include/linux/skbuff.h | 16 ++++++++--------
 net/core/skbuff.c      | 28 ++++++++++++++--------------
 net/ipv4/ip_output.c   |  2 +-
 net/ipv4/tcp.c         |  2 +-
 net/ipv6/ip6_output.c  |  2 +-
 5 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 9f7393167f0a..3c34c75ab004 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -488,13 +488,13 @@ struct ubuf_info {
 int mm_account_pinned_pages(struct mmpin *mmp, size_t size);
 void mm_unaccount_pinned_pages(struct mmpin *mmp);
 
-struct ubuf_info *sock_zerocopy_alloc(struct sock *sk, size_t size);
-struct ubuf_info *sock_zerocopy_realloc(struct sock *sk, size_t size,
-					struct ubuf_info *uarg);
+struct ubuf_info *msg_zerocopy_alloc(struct sock *sk, size_t size);
+struct ubuf_info *msg_zerocopy_realloc(struct sock *sk, size_t size,
+				       struct ubuf_info *uarg);
 
-void sock_zerocopy_put_abort(struct ubuf_info *uarg, bool have_uref);
+void msg_zerocopy_put_abort(struct ubuf_info *uarg, bool have_uref);
 
-void sock_zerocopy_callback(struct sk_buff *skb, struct ubuf_info *uarg,
+void msg_zerocopy_callback(struct sk_buff *skb, struct ubuf_info *uarg,
 			    bool success);
 
 int skb_zerocopy_iter_dgram(struct sk_buff *skb, struct msghdr *msg, int len);
@@ -1481,8 +1481,8 @@ static inline void skb_zcopy_put(struct ubuf_info *uarg)
 static inline void skb_zcopy_put_abort(struct ubuf_info *uarg, bool have_uref)
 {
 	if (uarg) {
-		if (uarg->callback == sock_zerocopy_callback)
-			sock_zerocopy_put_abort(uarg, have_uref);
+		if (uarg->callback == msg_zerocopy_callback)
+			msg_zerocopy_put_abort(uarg, have_uref);
 		else if (have_uref)
 			skb_zcopy_put(uarg);
 	}
@@ -2776,7 +2776,7 @@ static inline int skb_orphan_frags(struct sk_buff *skb, gfp_t gfp_mask)
 	if (likely(!skb_zcopy(skb)))
 		return 0;
 	if (!skb_zcopy_is_nouarg(skb) &&
-	    skb_uarg(skb)->callback == sock_zerocopy_callback)
+	    skb_uarg(skb)->callback == msg_zerocopy_callback)
 		return 0;
 	return skb_copy_ubufs(skb, gfp_mask);
 }
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 6d031ed99182..d520168accf9 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -1094,7 +1094,7 @@ void mm_unaccount_pinned_pages(struct mmpin *mmp)
 }
 EXPORT_SYMBOL_GPL(mm_unaccount_pinned_pages);
 
-struct ubuf_info *sock_zerocopy_alloc(struct sock *sk, size_t size)
+struct ubuf_info *msg_zerocopy_alloc(struct sock *sk, size_t size)
 {
 	struct ubuf_info *uarg;
 	struct sk_buff *skb;
@@ -1114,7 +1114,7 @@ struct ubuf_info *sock_zerocopy_alloc(struct sock *sk, size_t size)
 		return NULL;
 	}
 
-	uarg->callback = sock_zerocopy_callback;
+	uarg->callback = msg_zerocopy_callback;
 	uarg->id = ((u32)atomic_inc_return(&sk->sk_zckey)) - 1;
 	uarg->len = 1;
 	uarg->bytelen = size;
@@ -1124,15 +1124,15 @@ struct ubuf_info *sock_zerocopy_alloc(struct sock *sk, size_t size)
 
 	return uarg;
 }
-EXPORT_SYMBOL_GPL(sock_zerocopy_alloc);
+EXPORT_SYMBOL_GPL(msg_zerocopy_alloc);
 
 static inline struct sk_buff *skb_from_uarg(struct ubuf_info *uarg)
 {
 	return container_of((void *)uarg, struct sk_buff, cb);
 }
 
-struct ubuf_info *sock_zerocopy_realloc(struct sock *sk, size_t size,
-					struct ubuf_info *uarg)
+struct ubuf_info *msg_zerocopy_realloc(struct sock *sk, size_t size,
+				       struct ubuf_info *uarg)
 {
 	if (uarg) {
 		const u32 byte_limit = 1 << 19;		/* limit to a few TSO */
@@ -1171,9 +1171,9 @@ struct ubuf_info *sock_zerocopy_realloc(struct sock *sk, size_t size,
 	}
 
 new_alloc:
-	return sock_zerocopy_alloc(sk, size);
+	return msg_zerocopy_alloc(sk, size);
 }
-EXPORT_SYMBOL_GPL(sock_zerocopy_realloc);
+EXPORT_SYMBOL_GPL(msg_zerocopy_realloc);
 
 static bool skb_zerocopy_notify_extend(struct sk_buff *skb, u32 lo, u16 len)
 {
@@ -1195,7 +1195,7 @@ static bool skb_zerocopy_notify_extend(struct sk_buff *skb, u32 lo, u16 len)
 	return true;
 }
 
-static void __sock_zerocopy_callback(struct ubuf_info *uarg)
+static void __msg_zerocopy_callback(struct ubuf_info *uarg)
 {
 	struct sk_buff *tail, *skb = skb_from_uarg(uarg);
 	struct sock_exterr_skb *serr;
@@ -1243,17 +1243,17 @@ static void __sock_zerocopy_callback(struct ubuf_info *uarg)
 	sock_put(sk);
 }
 
-void sock_zerocopy_callback(struct sk_buff *skb, struct ubuf_info *uarg,
+void msg_zerocopy_callback(struct sk_buff *skb, struct ubuf_info *uarg,
 			    bool success)
 {
 	uarg->zerocopy = uarg->zerocopy & success;
 
 	if (refcount_dec_and_test(&uarg->refcnt))
-		__sock_zerocopy_callback(uarg);
+		__msg_zerocopy_callback(uarg);
 }
-EXPORT_SYMBOL_GPL(sock_zerocopy_callback);
+EXPORT_SYMBOL_GPL(msg_zerocopy_callback);
 
-void sock_zerocopy_put_abort(struct ubuf_info *uarg, bool have_uref)
+void msg_zerocopy_put_abort(struct ubuf_info *uarg, bool have_uref)
 {
 	struct sock *sk = skb_from_uarg(uarg)->sk;
 
@@ -1261,9 +1261,9 @@ void sock_zerocopy_put_abort(struct ubuf_info *uarg, bool have_uref)
 	uarg->len--;
 
 	if (have_uref)
-		sock_zerocopy_callback(NULL, uarg, true);
+		msg_zerocopy_callback(NULL, uarg, true);
 }
-EXPORT_SYMBOL_GPL(sock_zerocopy_put_abort);
+EXPORT_SYMBOL_GPL(msg_zerocopy_put_abort);
 
 int skb_zerocopy_iter_dgram(struct sk_buff *skb, struct msghdr *msg, int len)
 {
diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index 65f2299fd682..d8eb8b827794 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -1018,7 +1018,7 @@ static int __ip_append_data(struct sock *sk,
 		csummode = CHECKSUM_PARTIAL;
 
 	if (flags & MSG_ZEROCOPY && length && sock_flag(sk, SOCK_ZEROCOPY)) {
-		uarg = sock_zerocopy_realloc(sk, length, skb_zcopy(skb));
+		uarg = msg_zerocopy_realloc(sk, length, skb_zcopy(skb));
 		if (!uarg)
 			return -ENOBUFS;
 		extra_uref = !skb_zcopy(skb);	/* only ref on new uarg */
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index fb58215972ba..2882d520f5b1 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -1217,7 +1217,7 @@ int tcp_sendmsg_locked(struct sock *sk, struct msghdr *msg, size_t size)
 
 	if (flags & MSG_ZEROCOPY && size && sock_flag(sk, SOCK_ZEROCOPY)) {
 		skb = tcp_write_queue_tail(sk);
-		uarg = sock_zerocopy_realloc(sk, size, skb_zcopy(skb));
+		uarg = msg_zerocopy_realloc(sk, size, skb_zcopy(skb));
 		if (!uarg) {
 			err = -ENOBUFS;
 			goto out_err;
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index c8c87891533a..f59cfa39686a 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -1471,7 +1471,7 @@ static int __ip6_append_data(struct sock *sk,
 		csummode = CHECKSUM_PARTIAL;
 
 	if (flags & MSG_ZEROCOPY && length && sock_flag(sk, SOCK_ZEROCOPY)) {
-		uarg = sock_zerocopy_realloc(sk, length, skb_zcopy(skb));
+		uarg = msg_zerocopy_realloc(sk, length, skb_zcopy(skb));
 		if (!uarg)
 			return -ENOBUFS;
 		extra_uref = !skb_zcopy(skb);	/* only ref on new uarg */
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread
* [RFC PATCH v3 10/12] net: group skb_shinfo zerocopy related bits together.
  2020-12-30 19:12 [RFC PATCH v3 00/12] Generic zcopy_* functions Jonathan Lemon
                   ` (8 preceding siblings ...)
  2020-12-30 19:12 ` [RFC PATCH v3 09/12] skbuff: rename sock_zerocopy_* to msg_zerocopy_* Jonathan Lemon
@ 2020-12-30 19:12 ` Jonathan Lemon
  2020-12-30 19:12 ` [RFC PATCH v3 11/12] skbuff: add flags to ubuf_info for ubuf setup Jonathan Lemon
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 17+ messages in thread
From: Jonathan Lemon @ 2020-12-30 19:12 UTC (permalink / raw)
  To: netdev, willemdebruijn.kernel, edumazet, dsahern; +Cc: kernel-team

From: Jonathan Lemon <bsd@fb.com>

In preparation for expanded zerocopy (TX and RX), move
the zerocopy related bits out of tx_flags into their own
flag word.

Signed-off-by: Jonathan Lemon <jonathan.lemon@gmail.com>
---
 drivers/net/tap.c                   |  3 +--
 drivers/net/tun.c                   |  3 +--
 drivers/net/xen-netback/interface.c |  4 +--
 include/linux/skbuff.h              | 38 ++++++++++++++++-------------
 net/core/skbuff.c                   |  9 +++----
 net/ipv4/tcp.c                      |  2 +-
 net/kcm/kcmsock.c                   |  4 +--
 7 files changed, 32 insertions(+), 31 deletions(-)

diff --git a/drivers/net/tap.c b/drivers/net/tap.c
index 3f51f3766d18..f7a19d9b7c27 100644
--- a/drivers/net/tap.c
+++ b/drivers/net/tap.c
@@ -723,8 +723,7 @@ static ssize_t tap_get_user(struct tap_queue *q, void *msg_control,
 	/* copy skb_ubuf_info for callback when skb has no error */
 	if (zerocopy) {
 		skb_shinfo(skb)->destructor_arg = msg_control;
-		skb_shinfo(skb)->tx_flags |= SKBTX_DEV_ZEROCOPY;
-		skb_shinfo(skb)->tx_flags |= SKBTX_SHARED_FRAG;
+		skb_shinfo(skb)->flags |= SKBFL_ZEROCOPY_FRAG;
 	} else if (msg_control) {
 		struct ubuf_info *uarg = msg_control;
 		uarg->callback(NULL, uarg, false);
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index b64e2cc85751..dd9edbd72ae8 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -1815,8 +1815,7 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
 	/* copy skb_ubuf_info for callback when skb has no error */
 	if (zerocopy) {
 		skb_shinfo(skb)->destructor_arg = msg_control;
-		skb_shinfo(skb)->tx_flags |= SKBTX_DEV_ZEROCOPY;
-		skb_shinfo(skb)->tx_flags |= SKBTX_SHARED_FRAG;
+		skb_shinfo(skb)->flags |= SKBFL_ZEROCOPY_FRAG;
 	} else if (msg_control) {
 		struct ubuf_info *uarg = msg_control;
 		uarg->callback(NULL, uarg, false);
diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-netback/interface.c
index acb786d8b1d8..08b0e3d0b7eb 100644
--- a/drivers/net/xen-netback/interface.c
+++ b/drivers/net/xen-netback/interface.c
@@ -47,7 +47,7 @@
 /* Number of bytes allowed on the internal guest Rx queue. */
 #define XENVIF_RX_QUEUE_BYTES (XEN_NETIF_RX_RING_SIZE/2 * PAGE_SIZE)
 
-/* This function is used to set SKBTX_DEV_ZEROCOPY as well as
+/* This function is used to set SKBFL_ZEROCOPY_ENABLE as well as
  * increasing the inflight counter. We need to increase the inflight
  * counter because core driver calls into xenvif_zerocopy_callback
  * which calls xenvif_skb_zerocopy_complete.
@@ -55,7 +55,7 @@
 void xenvif_skb_zerocopy_prepare(struct xenvif_queue *queue,
 				 struct sk_buff *skb)
 {
-	skb_shinfo(skb)->tx_flags |= SKBTX_DEV_ZEROCOPY;
+	skb_shinfo(skb)->flags |= SKBFL_ZEROCOPY_ENABLE;
 	atomic_inc(&queue->inflight_packets);
 }
 
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 3c34c75ab004..66fde9a7b851 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -430,28 +430,32 @@ enum {
 	/* device driver is going to provide hardware time stamp */
 	SKBTX_IN_PROGRESS = 1 << 2,
 
-	/* device driver supports TX zero-copy buffers */
-	SKBTX_DEV_ZEROCOPY = 1 << 3,
-
 	/* generate wifi status information (where possible) */
 	SKBTX_WIFI_STATUS = 1 << 4,
 
-	/* This indicates at least one fragment might be overwritten
-	 * (as in vmsplice(), sendfile() ...)
-	 * If we need to compute a TX checksum, we'll need to copy
-	 * all frags to avoid possible bad checksum
-	 */
-	SKBTX_SHARED_FRAG = 1 << 5,
-
 	/* generate software time stamp when entering packet scheduling */
 	SKBTX_SCHED_TSTAMP = 1 << 6,
 };
 
-#define SKBTX_ZEROCOPY_FRAG	(SKBTX_DEV_ZEROCOPY | SKBTX_SHARED_FRAG)
 #define SKBTX_ANY_SW_TSTAMP	(SKBTX_SW_TSTAMP    | \
 				 SKBTX_SCHED_TSTAMP)
 #define SKBTX_ANY_TSTAMP	(SKBTX_HW_TSTAMP | SKBTX_ANY_SW_TSTAMP)
 
+/* Definitions for flags in struct skb_shared_info */
+enum {
+	/* use zcopy routines */
+	SKBFL_ZEROCOPY_ENABLE = BIT(0),
+
+	/* This indicates at least one fragment might be overwritten
+	 * (as in vmsplice(), sendfile() ...)
+	 * If we need to compute a TX checksum, we'll need to copy
+	 * all frags to avoid possible bad checksum
+	 */
+	SKBFL_SHARED_FRAG = BIT(1),
+};
+
+#define SKBFL_ZEROCOPY_FRAG	(SKBFL_ZEROCOPY_ENABLE | SKBFL_SHARED_FRAG)
+
 /*
  * The callback notifies userspace to release buffers when skb DMA is done in
  * lower device, the skb last reference should be 0 when calling this.
@@ -506,7 +510,7 @@ int skb_zerocopy_iter_stream(struct sock *sk, struct sk_buff *skb,
  * the end of the header data, ie. at skb->end.
  */
 struct skb_shared_info {
-	__u8		__unused;
+	__u8		flags;
 	__u8		meta_len;
 	__u8		nr_frags;
 	__u8		tx_flags;
@@ -1433,7 +1437,7 @@ static inline struct skb_shared_hwtstamps *skb_hwtstamps(struct sk_buff *skb)
 
 static inline struct ubuf_info *skb_zcopy(struct sk_buff *skb)
 {
-	bool is_zcopy = skb && skb_shinfo(skb)->tx_flags & SKBTX_DEV_ZEROCOPY;
+	bool is_zcopy = skb && skb_shinfo(skb)->flags & SKBFL_ZEROCOPY_ENABLE;
 
 	return is_zcopy ? skb_uarg(skb) : NULL;
 }
@@ -1452,14 +1456,14 @@ static inline void skb_zcopy_set(struct sk_buff *skb, struct ubuf_info *uarg,
 		else
 			skb_zcopy_get(uarg);
 		skb_shinfo(skb)->destructor_arg = uarg;
-		skb_shinfo(skb)->tx_flags |= SKBTX_ZEROCOPY_FRAG;
+		skb_shinfo(skb)->flags |= SKBFL_ZEROCOPY_FRAG;
 	}
 }
 
 static inline void skb_zcopy_set_nouarg(struct sk_buff *skb, void *val)
 {
 	skb_shinfo(skb)->destructor_arg = (void *)((uintptr_t) val | 0x1UL);
-	skb_shinfo(skb)->tx_flags |= SKBTX_ZEROCOPY_FRAG;
+	skb_shinfo(skb)->flags |= SKBFL_ZEROCOPY_FRAG;
 }
 
 static inline bool skb_zcopy_is_nouarg(struct sk_buff *skb)
@@ -1497,7 +1501,7 @@ static inline void skb_zcopy_clear(struct sk_buff *skb, bool zerocopy_success)
 		if (!skb_zcopy_is_nouarg(skb))
 			uarg->callback(skb, uarg, zerocopy_success);
 
-		skb_shinfo(skb)->tx_flags &= ~SKBTX_ZEROCOPY_FRAG;
+		skb_shinfo(skb)->flags &= ~SKBFL_ZEROCOPY_FRAG;
 	}
 }
 
@@ -3323,7 +3327,7 @@ static inline int skb_linearize(struct sk_buff *skb)
 static inline bool skb_has_shared_frag(const struct sk_buff *skb)
 {
 	return skb_is_nonlinear(skb) &&
-	       skb_shinfo(skb)->tx_flags & SKBTX_SHARED_FRAG;
+	       skb_shinfo(skb)->flags & SKBFL_SHARED_FRAG;
 }
 
 /**
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index d520168accf9..124e4752afb6 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -1327,7 +1327,7 @@ static int skb_zerocopy_clone(struct sk_buff *nskb, struct sk_buff *orig,
  *	@skb: the skb to modify
  *	@gfp_mask: allocation priority
  *
- *	This must be called on SKBTX_DEV_ZEROCOPY skb.
+ *	This must be called on skb with SKBFL_ZEROCOPY_ENABLE.
  *	It will copy all frags into kernel and drop the reference
  *	to userspace pages.
  *
@@ -3264,8 +3264,7 @@ void skb_split(struct sk_buff *skb, struct sk_buff *skb1, const u32 len)
 {
 	int pos = skb_headlen(skb);
 
-	skb_shinfo(skb1)->tx_flags |= skb_shinfo(skb)->tx_flags &
-				      SKBTX_SHARED_FRAG;
+	skb_shinfo(skb1)->flags |= skb_shinfo(skb)->flags & SKBFL_SHARED_FRAG;
 	skb_zerocopy_clone(skb1, skb, 0);
 	if (len < pos)	/* Split line is inside header. */
 		skb_split_inside_header(skb, skb1, len, pos);
@@ -3954,8 +3953,8 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
 		skb_copy_from_linear_data_offset(head_skb, offset,
 						 skb_put(nskb, hsize), hsize);
 
-		skb_shinfo(nskb)->tx_flags |= skb_shinfo(head_skb)->tx_flags &
-					      SKBTX_SHARED_FRAG;
+		skb_shinfo(nskb)->flags |= skb_shinfo(head_skb)->flags &
+					   SKBFL_SHARED_FRAG;
 
 		if (skb_orphan_frags(frag_skb, GFP_ATOMIC) ||
 		    skb_zerocopy_clone(nskb, frag_skb, GFP_ATOMIC))
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 2882d520f5b1..1954190b33c7 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -1010,7 +1010,7 @@ struct sk_buff *tcp_build_frag(struct sock *sk, int size_goal, int flags,
 	}
 
 	if (!(flags & MSG_NO_SHARED_FRAGS))
-		skb_shinfo(skb)->tx_flags |= SKBTX_SHARED_FRAG;
+		skb_shinfo(skb)->flags |= SKBFL_SHARED_FRAG;
 
 	skb->len += copy;
 	skb->data_len += copy;
diff --git a/net/kcm/kcmsock.c b/net/kcm/kcmsock.c
index 56dad9565bc9..35a13681c337 100644
--- a/net/kcm/kcmsock.c
+++ b/net/kcm/kcmsock.c
@@ -786,7 +786,7 @@ static ssize_t kcm_sendpage(struct socket *sock, struct page *page,
 
 		if (skb_can_coalesce(skb, i, page, offset)) {
 			skb_frag_size_add(&skb_shinfo(skb)->frags[i - 1], size);
-			skb_shinfo(skb)->tx_flags |= SKBTX_SHARED_FRAG;
+			skb_shinfo(skb)->flags |= SKBFL_SHARED_FRAG;
 			goto coalesced;
 		}
 
@@ -834,7 +834,7 @@ static ssize_t kcm_sendpage(struct socket *sock, struct page *page,
 
 	get_page(page);
 	skb_fill_page_desc(skb, i, page, offset, size);
-	skb_shinfo(skb)->tx_flags |= SKBTX_SHARED_FRAG;
+	skb_shinfo(skb)->flags |= SKBFL_SHARED_FRAG;
 
 coalesced:
 	skb->len += size;
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread
* [RFC PATCH v3 11/12] skbuff: add flags to ubuf_info for ubuf setup
  2020-12-30 19:12 [RFC PATCH v3 00/12] Generic zcopy_* functions Jonathan Lemon
                   ` (9 preceding siblings ...)
  2020-12-30 19:12 ` [RFC PATCH v3 10/12] net: group skb_shinfo zerocopy related bits together Jonathan Lemon
@ 2020-12-30 19:12 ` Jonathan Lemon
  2020-12-30 19:12 ` [RFC PATCH v3 12/12] tap/tun: add skb_zcopy_init() helper for initialization Jonathan Lemon
  2021-01-04 17:39 ` [RFC PATCH v3 00/12] Generic zcopy_* functions Willem de Bruijn
  12 siblings, 0 replies; 17+ messages in thread
From: Jonathan Lemon @ 2020-12-30 19:12 UTC (permalink / raw)
  To: netdev, willemdebruijn.kernel, edumazet, dsahern; +Cc: kernel-team

From: Jonathan Lemon <bsd@fb.com>

Currently, when an ubuf is attached to a new skb, the shared
flags word is initialized to a fixed value.  Instead of doing
this, set the default flags in the ubuf, and have new skbs
inherit from this default.

This is needed when setting up different zerocopy types.

Signed-off-by: Jonathan Lemon <jonathan.lemon@gmail.com>
---
 include/linux/skbuff.h | 3 ++-
 net/core/skbuff.c      | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 66fde9a7b851..58010df9d183 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -480,6 +480,7 @@ struct ubuf_info {
 		};
 	};
 	refcount_t refcnt;
+	u8 flags;
 
 	struct mmpin {
 		struct user_struct *user;
@@ -1456,7 +1457,7 @@ static inline void skb_zcopy_set(struct sk_buff *skb, struct ubuf_info *uarg,
 		else
 			skb_zcopy_get(uarg);
 		skb_shinfo(skb)->destructor_arg = uarg;
-		skb_shinfo(skb)->flags |= SKBFL_ZEROCOPY_FRAG;
+		skb_shinfo(skb)->flags |= uarg->flags;
 	}
 }
 
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 124e4752afb6..ee288af095f0 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -1119,6 +1119,7 @@ struct ubuf_info *msg_zerocopy_alloc(struct sock *sk, size_t size)
 	uarg->len = 1;
 	uarg->bytelen = size;
 	uarg->zerocopy = 1;
+	uarg->flags = SKBFL_ZEROCOPY_FRAG;
 	refcount_set(&uarg->refcnt, 1);
 	sock_hold(sk);
 
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread
* [RFC PATCH v3 12/12] tap/tun: add skb_zcopy_init() helper for initialization.
  2020-12-30 19:12 [RFC PATCH v3 00/12] Generic zcopy_* functions Jonathan Lemon
                   ` (10 preceding siblings ...)
  2020-12-30 19:12 ` [RFC PATCH v3 11/12] skbuff: add flags to ubuf_info for ubuf setup Jonathan Lemon
@ 2020-12-30 19:12 ` Jonathan Lemon
  2021-01-04 17:39 ` [RFC PATCH v3 00/12] Generic zcopy_* functions Willem de Bruijn
  12 siblings, 0 replies; 17+ messages in thread
From: Jonathan Lemon @ 2020-12-30 19:12 UTC (permalink / raw)
  To: netdev, willemdebruijn.kernel, edumazet, dsahern; +Cc: kernel-team

From: Jonathan Lemon <bsd@fb.com>

Replace direct assignments with skb_zcopy_init() for zerocopy
cases where a new skb is initialized, without changing the
reference counts.

Signed-off-by: Jonathan Lemon <jonathan.lemon@gmail.com>
---
 drivers/net/tap.c      | 3 +--
 drivers/net/tun.c      | 3 +--
 drivers/vhost/net.c    | 1 +
 include/linux/skbuff.h | 9 +++++++--
 4 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/net/tap.c b/drivers/net/tap.c
index f7a19d9b7c27..3c652c8ac5ba 100644
--- a/drivers/net/tap.c
+++ b/drivers/net/tap.c
@@ -722,8 +722,7 @@ static ssize_t tap_get_user(struct tap_queue *q, void *msg_control,
 	tap = rcu_dereference(q->tap);
 	/* copy skb_ubuf_info for callback when skb has no error */
 	if (zerocopy) {
-		skb_shinfo(skb)->destructor_arg = msg_control;
-		skb_shinfo(skb)->flags |= SKBFL_ZEROCOPY_FRAG;
+		skb_zcopy_init(skb, msg_control);
 	} else if (msg_control) {
 		struct ubuf_info *uarg = msg_control;
 		uarg->callback(NULL, uarg, false);
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index dd9edbd72ae8..7414e0584729 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -1814,8 +1814,7 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
 
 	/* copy skb_ubuf_info for callback when skb has no error */
 	if (zerocopy) {
-		skb_shinfo(skb)->destructor_arg = msg_control;
-		skb_shinfo(skb)->flags |= SKBFL_ZEROCOPY_FRAG;
+		skb_zcopy_init(skb, msg_control);
 	} else if (msg_control) {
 		struct ubuf_info *uarg = msg_control;
 		uarg->callback(NULL, uarg, false);
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index bf28d0b75c1b..5c722c4179a9 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -904,6 +904,7 @@ static void handle_tx_zerocopy(struct vhost_net *net, struct socket *sock)
 			ubuf->callback = vhost_zerocopy_callback;
 			ubuf->ctx = nvq->ubufs;
 			ubuf->desc = nvq->upend_idx;
+			ubuf->flags = SKBFL_ZEROCOPY_FRAG;
 			refcount_set(&ubuf->refcnt, 1);
 			msg.msg_control = &ctl;
 			ctl.type = TUN_MSG_UBUF;
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 58010df9d183..3ec8b83aca3e 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -1448,6 +1448,12 @@ static inline void skb_zcopy_get(struct ubuf_info *uarg)
 	refcount_inc(&uarg->refcnt);
 }
 
+static inline void skb_zcopy_init(struct sk_buff *skb, struct ubuf_info *uarg)
+{
+	skb_shinfo(skb)->destructor_arg = uarg;
+	skb_shinfo(skb)->flags |= uarg->flags;
+}
+
 static inline void skb_zcopy_set(struct sk_buff *skb, struct ubuf_info *uarg,
 				 bool *have_ref)
 {
@@ -1456,8 +1462,7 @@ static inline void skb_zcopy_set(struct sk_buff *skb, struct ubuf_info *uarg,
 			*have_ref = false;
 		else
 			skb_zcopy_get(uarg);
-		skb_shinfo(skb)->destructor_arg = uarg;
-		skb_shinfo(skb)->flags |= uarg->flags;
+		skb_zcopy_init(skb, uarg);
 	}
 }
 
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread
* Re: [RFC PATCH v3 00/12] Generic zcopy_* functions
  2020-12-30 19:12 [RFC PATCH v3 00/12] Generic zcopy_* functions Jonathan Lemon
                   ` (11 preceding siblings ...)
  2020-12-30 19:12 ` [RFC PATCH v3 12/12] tap/tun: add skb_zcopy_init() helper for initialization Jonathan Lemon
@ 2021-01-04 17:39 ` Willem de Bruijn
  2021-01-05  4:17   ` Jonathan Lemon
  12 siblings, 1 reply; 17+ messages in thread
From: Willem de Bruijn @ 2021-01-04 17:39 UTC (permalink / raw)
  To: Jonathan Lemon
  Cc: Network Development, Eric Dumazet, David Ahern, Kernel Team

On Wed, Dec 30, 2020 at 2:12 PM Jonathan Lemon <jonathan.lemon@gmail.com> wrote:
>
> From: Jonathan Lemon <bsd@fb.com>
>
> This is set of cleanup patches for zerocopy which are intended
> to allow a introduction of a different zerocopy implementation.
>
> The top level API will use the skb_zcopy_*() functions, while
> the current TCP specific zerocopy ends up using msg_zerocopy_*()
> calls.
>
> There should be no functional changes from these patches.
>
> v2->v3:
>  Rename zc_flags to 'flags'.  Use SKBFL_xxx naming, similar
>  to the SKBTX_xx naming.  Leave zerocopy_success naming alone.
>  Reorder patches.
>
> v1->v2:
>  Break changes to skb_zcopy_put into 3 patches, in order to
>  make it easier to follow the changes.  Add Willem's suggestion
>  about renaming sock_zerocopy_

Overall, this latest version looks fine to me.

The big question is how this fits in with the broader rx direct
placement feature. But it makes sense to me to checkpoint as is at
this point.

One small comment: skb_zcopy_* is a logical prefix for functions that
act on sk_buffs, Such as skb_zcopy_set, which associates a uarg with
an skb. Less for functions that operate directly on the uarg, and do
not even take an skb as argument: skb_zcopy_get and skb_zcopy_put.
Perhaps net_zcopy_get/net_zcopy_put?

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [RFC PATCH v3 00/12] Generic zcopy_* functions
  2021-01-04 17:39 ` [RFC PATCH v3 00/12] Generic zcopy_* functions Willem de Bruijn
@ 2021-01-05  4:17   ` Jonathan Lemon
  2021-01-05  4:22     ` Willem de Bruijn
  0 siblings, 1 reply; 17+ messages in thread
From: Jonathan Lemon @ 2021-01-05  4:17 UTC (permalink / raw)
  To: Willem de Bruijn
  Cc: Network Development, Eric Dumazet, David Ahern, Kernel Team

On Mon, Jan 04, 2021 at 12:39:35PM -0500, Willem de Bruijn wrote:
> On Wed, Dec 30, 2020 at 2:12 PM Jonathan Lemon <jonathan.lemon@gmail.com> wrote:
> >
> > From: Jonathan Lemon <bsd@fb.com>
> >
> > This is set of cleanup patches for zerocopy which are intended
> > to allow a introduction of a different zerocopy implementation.
> >
> > The top level API will use the skb_zcopy_*() functions, while
> > the current TCP specific zerocopy ends up using msg_zerocopy_*()
> > calls.
> >
> > There should be no functional changes from these patches.
> >
> > v2->v3:
> >  Rename zc_flags to 'flags'.  Use SKBFL_xxx naming, similar
> >  to the SKBTX_xx naming.  Leave zerocopy_success naming alone.
> >  Reorder patches.
> >
> > v1->v2:
> >  Break changes to skb_zcopy_put into 3 patches, in order to
> >  make it easier to follow the changes.  Add Willem's suggestion
> >  about renaming sock_zerocopy_
> 
> Overall, this latest version looks fine to me.
> 
> The big question is how this fits in with the broader rx direct
> placement feature. But it makes sense to me to checkpoint as is at
> this point.
> 
> One small comment: skb_zcopy_* is a logical prefix for functions that
> act on sk_buffs, Such as skb_zcopy_set, which associates a uarg with
> an skb. Less for functions that operate directly on the uarg, and do
> not even take an skb as argument: skb_zcopy_get and skb_zcopy_put.
> Perhaps net_zcopy_get/net_zcopy_put?

Or even just zcopy_get / zcopy_put ?
-- 
Jonathan

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [RFC PATCH v3 00/12] Generic zcopy_* functions
  2021-01-05  4:17   ` Jonathan Lemon
@ 2021-01-05  4:22     ` Willem de Bruijn
  2021-01-05  5:38       ` Jonathan Lemon
  0 siblings, 1 reply; 17+ messages in thread
From: Willem de Bruijn @ 2021-01-05  4:22 UTC (permalink / raw)
  To: Jonathan Lemon
  Cc: Network Development, Eric Dumazet, David Ahern, Kernel Team

On Mon, Jan 4, 2021 at 11:17 PM Jonathan Lemon <jonathan.lemon@gmail.com> wrote:
>
> On Mon, Jan 04, 2021 at 12:39:35PM -0500, Willem de Bruijn wrote:
> > On Wed, Dec 30, 2020 at 2:12 PM Jonathan Lemon <jonathan.lemon@gmail.com> wrote:
> > >
> > > From: Jonathan Lemon <bsd@fb.com>
> > >
> > > This is set of cleanup patches for zerocopy which are intended
> > > to allow a introduction of a different zerocopy implementation.
> > >
> > > The top level API will use the skb_zcopy_*() functions, while
> > > the current TCP specific zerocopy ends up using msg_zerocopy_*()
> > > calls.
> > >
> > > There should be no functional changes from these patches.
> > >
> > > v2->v3:
> > >  Rename zc_flags to 'flags'.  Use SKBFL_xxx naming, similar
> > >  to the SKBTX_xx naming.  Leave zerocopy_success naming alone.
> > >  Reorder patches.
> > >
> > > v1->v2:
> > >  Break changes to skb_zcopy_put into 3 patches, in order to
> > >  make it easier to follow the changes.  Add Willem's suggestion
> > >  about renaming sock_zerocopy_
> >
> > Overall, this latest version looks fine to me.
> >
> > The big question is how this fits in with the broader rx direct
> > placement feature. But it makes sense to me to checkpoint as is at
> > this point.
> >
> > One small comment: skb_zcopy_* is a logical prefix for functions that
> > act on sk_buffs, Such as skb_zcopy_set, which associates a uarg with
> > an skb. Less for functions that operate directly on the uarg, and do
> > not even take an skb as argument: skb_zcopy_get and skb_zcopy_put.
> > Perhaps net_zcopy_get/net_zcopy_put?
>
> Or even just zcopy_get / zcopy_put ?

Zerocopy is such an overloaded term, that I'd keep some prefix, at least.

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [RFC PATCH v3 00/12] Generic zcopy_* functions
  2021-01-05  4:22     ` Willem de Bruijn
@ 2021-01-05  5:38       ` Jonathan Lemon
  0 siblings, 0 replies; 17+ messages in thread
From: Jonathan Lemon @ 2021-01-05  5:38 UTC (permalink / raw)
  To: Willem de Bruijn
  Cc: Network Development, Eric Dumazet, David Ahern, Kernel Team

On Mon, Jan 04, 2021 at 11:22:35PM -0500, Willem de Bruijn wrote:
> On Mon, Jan 4, 2021 at 11:17 PM Jonathan Lemon <jonathan.lemon@gmail.com> wrote:
> >
> > On Mon, Jan 04, 2021 at 12:39:35PM -0500, Willem de Bruijn wrote:
> > > On Wed, Dec 30, 2020 at 2:12 PM Jonathan Lemon <jonathan.lemon@gmail.com> wrote:
> > > >
> > > > From: Jonathan Lemon <bsd@fb.com>
> > > >
> > > > This is set of cleanup patches for zerocopy which are intended
> > > > to allow a introduction of a different zerocopy implementation.
> > > >
> > > > The top level API will use the skb_zcopy_*() functions, while
> > > > the current TCP specific zerocopy ends up using msg_zerocopy_*()
> > > > calls.
> > > >
> > > > There should be no functional changes from these patches.
> > > >
> > > > v2->v3:
> > > >  Rename zc_flags to 'flags'.  Use SKBFL_xxx naming, similar
> > > >  to the SKBTX_xx naming.  Leave zerocopy_success naming alone.
> > > >  Reorder patches.
> > > >
> > > > v1->v2:
> > > >  Break changes to skb_zcopy_put into 3 patches, in order to
> > > >  make it easier to follow the changes.  Add Willem's suggestion
> > > >  about renaming sock_zerocopy_
> > >
> > > Overall, this latest version looks fine to me.
> > >
> > > The big question is how this fits in with the broader rx direct
> > > placement feature. But it makes sense to me to checkpoint as is at
> > > this point.
> > >
> > > One small comment: skb_zcopy_* is a logical prefix for functions that
> > > act on sk_buffs, Such as skb_zcopy_set, which associates a uarg with
> > > an skb. Less for functions that operate directly on the uarg, and do
> > > not even take an skb as argument: skb_zcopy_get and skb_zcopy_put.
> > > Perhaps net_zcopy_get/net_zcopy_put?
> >
> > Or even just zcopy_get / zcopy_put ?
> 
> Zerocopy is such an overloaded term, that I'd keep some prefix, at least.

I'll make that change and repost the series when net-next opens.
--
Jonathan

^ permalink raw reply	[flat|nested] 17+ messages in thread
end of thread, other threads:[~2021-01-05  5:39 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-30 19:12 [RFC PATCH v3 00/12] Generic zcopy_* functions Jonathan Lemon
2020-12-30 19:12 ` [RFC PATCH v3 01/12] skbuff: remove unused skb_zcopy_abort function Jonathan Lemon
2020-12-30 19:12 ` [RFC PATCH v3 02/12] skbuff: simplify sock_zerocopy_put Jonathan Lemon
2020-12-30 19:12 ` [RFC PATCH v3 03/12] skbuff: Push status and refcounts into sock_zerocopy_callback Jonathan Lemon
2020-12-30 19:12 ` [RFC PATCH v3 04/12] skbuff: replace sock_zerocopy_put() with skb_zcopy_put() Jonathan Lemon
2020-12-30 19:12 ` [RFC PATCH v3 05/12] skbuff: replace sock_zerocopy_get with skb_zcopy_get Jonathan Lemon
2020-12-30 19:12 ` [RFC PATCH v3 06/12] skbuff: Add skb parameter to the ubuf zerocopy callback Jonathan Lemon
2020-12-30 19:12 ` [RFC PATCH v3 07/12] skbuff: Call sock_zerocopy_put_abort from skb_zcopy_put_abort Jonathan Lemon
2020-12-30 19:12 ` [RFC PATCH v3 08/12] skbuff: Call skb_zcopy_clear() before unref'ing fragments Jonathan Lemon
2020-12-30 19:12 ` [RFC PATCH v3 09/12] skbuff: rename sock_zerocopy_* to msg_zerocopy_* Jonathan Lemon
2020-12-30 19:12 ` [RFC PATCH v3 10/12] net: group skb_shinfo zerocopy related bits together Jonathan Lemon
2020-12-30 19:12 ` [RFC PATCH v3 11/12] skbuff: add flags to ubuf_info for ubuf setup Jonathan Lemon
2020-12-30 19:12 ` [RFC PATCH v3 12/12] tap/tun: add skb_zcopy_init() helper for initialization Jonathan Lemon
2021-01-04 17:39 ` [RFC PATCH v3 00/12] Generic zcopy_* functions Willem de Bruijn
2021-01-05  4:17   ` Jonathan Lemon
2021-01-05  4:22     ` Willem de Bruijn
2021-01-05  5:38       ` Jonathan Lemon

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