[openib-general] [PATCH] uMAD: remove receive side data copy

Sean Hefty sean.hefty at intel.com
Tue Feb 28 12:11:01 PST 2006


Remove the additional data copy when handing a received MAD up to userspace.
Only a single data copy to the user's buffer is now performed.  Checks for
the correct userspace buffer size are also tightened.

Signed-off-by: Sean Hefty <sean.hefty at intel.com>

---

NOTE: I tested this on a node running opensm, but I have no way of testing that
received RMPP MADs are copied correctly.


Index: user_mad.c
===================================================================
--- user_mad.c	(revision 5532)
+++ user_mad.c	(working copy)
@@ -121,9 +121,9 @@ struct ib_umad_file {
 
 struct ib_umad_packet {
 	struct ib_mad_send_buf *msg;
+	struct ib_mad_recv_wc  *recv_wc;
 	struct list_head   list;
 	int		   length;
-	struct list_head   seg_list;
 	struct ib_user_mad mad;
 };
 
@@ -188,62 +188,6 @@ static int data_offset(u8 mgmt_class)
 		return IB_MGMT_RMPP_HDR;
 }
 
-static int copy_recv_mad(struct ib_mad_recv_wc *mad_recv_wc,
-			 struct ib_umad_packet *packet)
-{
-	struct ib_mad_recv_buf *seg_buf;
-	struct ib_rmpp_mad *rmpp_mad;
-	void *data;
-	struct ib_rmpp_segment *seg;
-	int size, len, offset;
-	u8 flags;
-
-	len = mad_recv_wc->mad_len;
-	if (len <= sizeof(struct ib_mad)) {
-		memcpy(&packet->mad.data, mad_recv_wc->recv_buf.mad, len);
-		return 0;
-	}
-
-	/* Multipacket (RMPP) MAD */
-	offset = data_offset(mad_recv_wc->recv_buf.mad->mad_hdr.mgmt_class);
-
-	list_for_each_entry(seg_buf, &mad_recv_wc->rmpp_list, list) {
-		rmpp_mad = (struct ib_rmpp_mad *) seg_buf->mad;
-		flags = ib_get_rmpp_flags(&rmpp_mad->rmpp_hdr);
-
-		if (flags & IB_MGMT_RMPP_FLAG_FIRST) {
-			size = sizeof(*rmpp_mad);
-			memcpy(&packet->mad.data, rmpp_mad, size);
-		} else {
-			data = (void *) rmpp_mad + offset;
-			if (flags & IB_MGMT_RMPP_FLAG_LAST)
-				size = len;
-			else
-				size = sizeof(*rmpp_mad) - offset;
-			seg = kmalloc(sizeof(struct ib_rmpp_segment) +
-				      sizeof(struct ib_rmpp_mad) - offset,
-				      GFP_KERNEL);
-			if (!seg)
-				return -ENOMEM;
-			memcpy(seg->data, data, size);
-			list_add_tail(&seg->list, &packet->seg_list);
-		}
-		len -= size;
-	}
-	return 0;
-}
-
-static void free_packet(struct ib_umad_packet *packet)
-{
-	struct ib_rmpp_segment *seg, *tmp;
-
-	list_for_each_entry_safe(seg, tmp, &packet->seg_list, list) {
-		list_del(&seg->list);
-		kfree(seg);
-	}
-	kfree(packet);
-}
-
 static void send_handler(struct ib_mad_agent *agent,
 			 struct ib_mad_send_wc *send_wc)
 {
@@ -267,25 +211,20 @@ static void recv_handler(struct ib_mad_a
 {
 	struct ib_umad_file *file = agent->context;
 	struct ib_umad_packet *packet;
-	int length;
 
 	if (mad_recv_wc->wc->status != IB_WC_SUCCESS)
-		goto out;
+		goto err1;
 
-	length = mad_recv_wc->mad_len;
-	packet = kzalloc(sizeof *packet + sizeof(struct ib_mad), GFP_KERNEL);
+	packet = kzalloc(sizeof *packet, GFP_KERNEL);
 	if (!packet)
-		goto out;
-	INIT_LIST_HEAD(&packet->seg_list);
-	packet->length = length;
+		goto err1;
 
-	if (copy_recv_mad(mad_recv_wc, packet)) {
-		free_packet(packet);
-		goto out;
-	}
+	packet->length = mad_recv_wc->mad_len;
+	packet->recv_wc = mad_recv_wc;
 
 	packet->mad.hdr.status    = 0;
-	packet->mad.hdr.length    = length + sizeof (struct ib_user_mad);
+	packet->mad.hdr.length    = sizeof (struct ib_user_mad) +
+				    mad_recv_wc->mad_len;
 	packet->mad.hdr.qpn 	  = cpu_to_be32(mad_recv_wc->wc->src_qp);
 	packet->mad.hdr.lid 	  = cpu_to_be16(mad_recv_wc->wc->slid);
 	packet->mad.hdr.sl  	  = mad_recv_wc->wc->sl;
@@ -301,21 +240,87 @@ static void recv_handler(struct ib_mad_a
 	}
 
 	if (queue_packet(file, agent, packet))
-		free_packet(packet);
+		goto err2;
+	return;
 
-out:
+err2:
+	kfree(packet);
+err1:
 	ib_free_recv_mad(mad_recv_wc);
 }
 
+static ssize_t copy_recv_mad(char __user *buf, struct ib_umad_packet *packet,
+			     size_t count)
+{
+	struct ib_mad_recv_buf *recv_buf;
+	int left, seg_payload, offset, max_seg_payload;
+
+	/* We need enough room to copy the first (or only) MAD segment. */
+	recv_buf = &packet->recv_wc->recv_buf;
+	if ((packet->length <= sizeof (*recv_buf->mad) &&
+	     count < sizeof (packet->mad) + packet->length) ||
+	    (packet->length > sizeof (*recv_buf->mad) &&
+	     count < sizeof (packet->mad) + sizeof (*recv_buf->mad)))
+		return -EINVAL;
+
+	if (copy_to_user(buf, &packet->mad, sizeof (packet->mad)))
+		return -EFAULT;
+
+	buf += sizeof (packet->mad);
+	seg_payload = min_t(int, packet->length, sizeof (*recv_buf->mad));
+	if (copy_to_user(buf, recv_buf->mad, seg_payload))
+		return -EFAULT;
+
+	if (seg_payload < packet->length) {
+		/*
+		 * Multipacket RMPP MAD message. Copy remainder of message.
+		 * Note that last segment may have a shorter payload.
+		 */
+		if (count < sizeof (packet->mad) + packet->length) {
+			/*
+			 * The buffer is too small, return the first RMPP segment,
+			 * which includes the RMPP message length.
+			 */
+			return -ENOSPC;
+		}
+		offset = data_offset(recv_buf->mad->mad_hdr.mgmt_class);
+		max_seg_payload = sizeof (struct ib_mad) - offset;
+
+		for (left = packet->length - seg_payload, buf += seg_payload;
+		     left; left -= seg_payload, buf += seg_payload) {
+			recv_buf = container_of(recv_buf->list.next,
+						struct ib_mad_recv_buf, list);
+			seg_payload = min(left, max_seg_payload);
+			if (copy_to_user(buf, ((void *) recv_buf->mad) + offset,
+					 seg_payload))
+				return -EFAULT;
+		}
+	}
+	return sizeof (packet->mad) + packet->length;
+}
+
+static ssize_t copy_send_mad(char __user *buf, struct ib_umad_packet *packet,
+			     size_t count)
+{
+	ssize_t size = sizeof (packet->mad) + packet->length;
+
+	if (count < size)
+		return -EINVAL;
+
+	if (copy_to_user(buf, &packet->mad, size))
+		return -EFAULT;
+
+	return size;
+}
+
 static ssize_t ib_umad_read(struct file *filp, char __user *buf,
 			    size_t count, loff_t *pos)
 {
 	struct ib_umad_file *file = filp->private_data;
-	struct ib_rmpp_segment *seg;
 	struct ib_umad_packet *packet;
-	ssize_t ret, size;
+	ssize_t ret;
 
-	if (count < sizeof (struct ib_user_mad) + sizeof (struct ib_mad))
+	if (count < sizeof (struct ib_user_mad))
 		return -EINVAL;
 
 	spin_lock_irq(&file->recv_lock);
@@ -338,52 +343,21 @@ static ssize_t ib_umad_read(struct file 
 
 	spin_unlock_irq(&file->recv_lock);
 
-	size = min_t(int, sizeof (struct ib_mad), packet->length);
-	if (copy_to_user(buf, &packet->mad,
-			 sizeof(struct ib_user_mad) + size)) {
-		ret = -EFAULT;
-		goto err;
-	}
+	if (packet->recv_wc)
+		ret = copy_recv_mad(buf, packet, count);
+	else
+		ret = copy_send_mad(buf, packet, count);
 
-	if (count < packet->length + sizeof (struct ib_user_mad))
-		/*
-		 * User buffer too small. Return first RMPP segment (which
-		 * includes RMPP message length).
-		 */
-		ret = -ENOSPC;
-	else if (packet->length <= sizeof(struct ib_mad))
-		ret = packet->length + sizeof(struct ib_user_mad);
-	else {
-		int len = packet->length - sizeof(struct ib_mad);
-		struct ib_rmpp_mad *rmpp_mad =
-				(struct ib_rmpp_mad *) packet->mad.data;
-		int max_seg_payload = sizeof(struct ib_mad) -
-				      data_offset(rmpp_mad->mad_hdr.mgmt_class);
-		int seg_payload;
-		/*
-		 * Multipacket RMPP MAD message. Copy remainder of message.
-		 * Note that last segment may have a shorter payload.
-		 */
-		buf += sizeof(struct ib_user_mad) + sizeof(struct ib_mad);
-		list_for_each_entry(seg, &packet->seg_list, list) {
-			seg_payload = min_t(int, len, max_seg_payload);
-			if (copy_to_user(buf, seg->data, seg_payload)) {
-				ret = -EFAULT;
-				goto err;
-			}
-			buf += seg_payload;
-			len -= seg_payload;
-		}
-		ret = packet->length + sizeof (struct ib_user_mad);
-	}
-err:
 	if (ret < 0) {
 		/* Requeue packet */
 		spin_lock_irq(&file->recv_lock);
 		list_add(&packet->list, &file->recv_list);
 		spin_unlock_irq(&file->recv_lock);
-	} else
-		free_packet(packet);
+	} else {
+		if (packet->recv_wc)
+			ib_free_recv_mad(packet->recv_wc);
+		kfree(packet);
+	}
 	return ret;
 }
 






More information about the general mailing list