[ovs-dev] [PATCH v2] datapath: Orphan frags in skb_zerocopy and handle errors

[Zoltan Kiss]

This is the ported version of commit 36d5fe6a with the same name from net-next.
Apart from the small datapath.c changes it adjust the compat layer files as
well. This is the original commit message:

"skb_zerocopy can copy elements of the frags array between skbs, but it doesn't
orphan them. Also, it doesn't handle errors, so this patch takes care of that
as well, and modify the callers accordingly. skb_tx_error() is also added to
the callers so they will signal the failed delivery towards the creator of the
skb."

Signed-off-by: Zoltan Kiss <zoltan.kiss at citrix.com>
---
v2:
- adding stubs of skb_tx_error() and skb_orphan_frags() for kernels older than
  3.8

diff --git a/datapath/datapath.c b/datapath/datapath.c
index 25edd7d..5e5aa71 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -485,7 +485,9 @@ static int queue_userspace_packet(struct datapath *dp, struct sk_buff *skb,
 	}
 	nla->nla_len = nla_attr_size(skb->len);
 
-	skb_zerocopy(user_skb, skb, skb->len, hlen);
+	err = skb_zerocopy(user_skb, skb, skb->len, hlen);
+	if (err)
+		goto out;
 
 	/* Pad OVS_PACKET_ATTR_PACKET if linear copy was performed */
 	if (!(dp->user_features & OVS_DP_F_UNALIGNED)) {
@@ -499,6 +501,8 @@ static int queue_userspace_packet(struct datapath *dp, struct sk_buff *skb,
 
 	err = genlmsg_unicast(ovs_dp_get_net(dp), user_skb, upcall_info->portid);
 out:
+	if (err)
+		skb_tx_error(skb);
 	kfree_skb(nskb);
 	return err;
 }
diff --git a/datapath/linux/compat/include/linux/skbuff.h b/datapath/linux/compat/include/linux/skbuff.h
index de0c56a..f46cbf2 100644
--- a/datapath/linux/compat/include/linux/skbuff.h
+++ b/datapath/linux/compat/include/linux/skbuff.h
@@ -256,11 +256,21 @@ static inline __u32 skb_get_rxhash(struct sk_buff *skb)
 #endif
 	return __skb_get_rxhash(skb);
 }
-#endif
+
+static inline void skb_tx_error(struct sk_buff *skb)
+{
+	return;
+}
+
+static inline int skb_orphan_frags(struct sk_buff *skb, gfp_t gfp_mask)
+{
+	return 0;
+}
+#endif /* LINUX_VERSION_CODE < KERNEL_VERSION(3,8,0) */
 
 #if LINUX_VERSION_CODE < KERNEL_VERSION(3,14,0)
 unsigned int skb_zerocopy_headlen(const struct sk_buff *from);
-void skb_zerocopy(struct sk_buff *to, const struct sk_buff *from, int len, 
+int skb_zerocopy(struct sk_buff *to, struct sk_buff *from, int len,
 		  int hlen);
 #endif
 
diff --git a/datapath/linux/compat/skbuff-openvswitch.c b/datapath/linux/compat/skbuff-openvswitch.c
index ddd7bc8..96c27d0 100644
--- a/datapath/linux/compat/skbuff-openvswitch.c
+++ b/datapath/linux/compat/skbuff-openvswitch.c
@@ -61,25 +61,31 @@ skb_zerocopy_headlen(const struct sk_buff *from)
  *
  *	The `hlen` as calculated by skb_zerocopy_headlen() specifies the
  *	headroom in the `to` buffer.
+ *
+ *	Return value:
+ *	0: everything is OK
+ *	-ENOMEM: couldn't orphan frags of @from due to lack of memory
+ *	-EFAULT: skb_copy_bits() found some problem with skb geometry
  */
-void
-skb_zerocopy(struct sk_buff *to, const struct sk_buff *from, int len, int hlen)
+int
+skb_zerocopy(struct sk_buff *to, struct sk_buff *from, int len, int hlen)
 {
 	int i, j = 0;
 	int plen = 0; /* length of skb->head fragment */
+	int ret;
 	struct page *page;
 	unsigned int offset;
 
 	BUG_ON(!head_frag(from) && !hlen);
 
 	/* dont bother with small payloads */
-	if (len <= skb_tailroom(to)) {
-		skb_copy_bits(from, 0, skb_put(to, len), len);
-		return;
-	}
+	if (len <= skb_tailroom(to))
+		return skb_copy_bits(from, 0, skb_put(to, len), len);
 
 	if (hlen) {
-		skb_copy_bits(from, 0, skb_put(to, hlen), hlen);
+		ret = skb_copy_bits(from, 0, skb_put(to, hlen), hlen);
+		if (unlikely(ret))
+			return ret;
 		len -= hlen;
 	} else {
 		plen = min_t(int, skb_headlen(from), len);
@@ -97,6 +103,11 @@ skb_zerocopy(struct sk_buff *to, const struct sk_buff *from, int len, int hlen)
 	to->len += len + plen;
 	to->data_len += len + plen;
 
+	if (unlikely(skb_orphan_frags(from, GFP_ATOMIC))) {
+		skb_tx_error(from);
+		return -ENOMEM;
+	}
+
 	for (i = 0; i < skb_shinfo(from)->nr_frags; i++) {
 		if (!len)
 			break;
@@ -107,5 +118,7 @@ skb_zerocopy(struct sk_buff *to, const struct sk_buff *from, int len, int hlen)
 		j++;
 	}
 	skb_shinfo(to)->nr_frags = j;
+
+	return 0;
 }
 #endif