[ofw] [PATCH 12/?] Clean up HCA driver

Fab Tillier ftillier at microsoft.com
Mon Jun 18 20:03:43 PDT 2012


This patch does the following:
- Use existing IN6_ADDR structure, rather than redefining it.
- Use existing macros for checking link-local or multicast IN6_ADDRs.
- Don't treat output handles to verbs calls as optional.  It is a programming error to pass NULL for the output handle, and the current code just quietly leaks the resource.
- Minor code syntax cleanup, like using curlies consistently
- Proper use of INIT_UDATA
- Proper use of user-provided parameters for QP creation (Why in the world are these duplicated in the verbs data?)

Signed-off-by: Fab Tillier <ftillier at microsoft.com>

Index: hw/mlx4/kernel/bus/ib/ah.c
===================================================================
--- hw/mlx4/kernel/bus/ib/ah.c	(revision 3414)
+++ hw/mlx4/kernel/bus/ib/ah.c	(working copy)
@@ -40,60 +40,33 @@
 #include "ah.tmh"
 #endif
 
-
-static inline int rdma_link_local_addr(struct ib_in6_addr *addr)
+static inline void rdma_get_mcast_mac(const union ib_gid *gid, u8 *mac)
 {
-	if (addr->s6_addr32[0] == cpu_to_be32(0xfe800000) &&
-	    addr->s6_addr32[1] == 0)
-		return 1;
-	else
-		return 0;
-}
-
-inline void rdma_get_ll_mac(struct ib_in6_addr *addr, u8 *mac)
-{
-	memcpy(mac, &addr->s6_addr[8], 3);
-	memcpy(mac + 3, &addr->s6_addr[13], 3);
-	mac[0] ^= 2;   
-}
-
-static inline int rdma_is_multicast_addr(struct ib_in6_addr *addr)
-{
-	return addr->s6_addr[0] == 0xff ? 1 : 0;
-}
-
-static inline void rdma_get_mcast_mac(struct ib_in6_addr *addr, u8 *mac)
-{
 	int i;
 
 	mac[0] = 0x33;
 	mac[1] = 0x33;
 	for (i = 2; i < 6; ++i)
-		mac[i] = addr->s6_addr[i + 10];
+		mac[i] = gid->raw[i + 10];
 
 }
 
 int mlx4_ib_resolve_grh(struct mlx4_ib_dev *dev, const struct ib_ah_attr *ah_attr,
 			u8 *mac, int *is_mcast)
 {
-	int err = 0;
-	struct ib_sockaddr_in6 dst;
-
 	UNREFERENCED_PARAMETER(dev);
 
+	if (IN6_IS_ADDR_LINKLOCAL((IN6_ADDR*)ah_attr->grh.dgid.raw)) {
+		rdma_get_ll_mac(&ah_attr->grh.dgid, mac);
 	*is_mcast = 0;
-	memcpy(dst.sin6_addr.s6_addr, ah_attr->grh.dgid.raw, sizeof(ah_attr->grh.dgid.raw));
-
-	if (rdma_link_local_addr(&dst.sin6_addr))
-		rdma_get_ll_mac(&dst.sin6_addr, mac);
-	else if (rdma_is_multicast_addr(&dst.sin6_addr)) {
-		rdma_get_mcast_mac(&dst.sin6_addr, mac);
+	} else if (IN6_IS_ADDR_MULTICAST((IN6_ADDR*)ah_attr->grh.dgid.raw)) {
+		rdma_get_mcast_mac(&ah_attr->grh.dgid, mac);
 		*is_mcast = 1;
 	} else {
-		err = -EINVAL; //jyang:todo
 		ASSERT(FALSE);
+		return -EINVAL; //jyang:todo
 	}
-	return err;
+	return 0;
 }
 
 static struct ib_ah *create_ib_ah(struct ib_pd *pd, struct ib_ah_attr *ah_attr,
Index: hw/mlx4/kernel/bus/inc/device.h
===================================================================
--- hw/mlx4/kernel/bus/inc/device.h	(revision 3414)
+++ hw/mlx4/kernel/bus/inc/device.h	(working copy)
@@ -118,11 +118,11 @@
 	MLX4_EVENT_TYPE_SRQ_LIMIT		   = 0x14,
 	MLX4_EVENT_TYPE_COMM_CHANNEL	   = 0x18,
 	MLX4_EVENT_TYPE_VEP_UPDATE		   = 0x19,
+	MLX4_EVENT_TYPE_OP_REQUIRED	   = 0x1a,
 	MLX4_EVENT_TYPE_MAC_UPDATE		   = 0x20,
 	MLX4_EVENT_TYPE_PPF_REMOVE		   = 0xf0,
 	MLX4_EVENT_TYPE_SQP_UPDATE	   = 0xfe,
-	MLX4_EVENT_TYPE_NONE		   = 0xff,
-	MLX4_EVENT_TYPE_OP_REQUIRED	   = 0x1a
+	MLX4_EVENT_TYPE_NONE		   = 0xff
 };
 
 enum {
Index: hw/mlx4/kernel/hca/cq.c
===================================================================
--- hw/mlx4/kernel/hca/cq.c	(revision 3414)
+++ hw/mlx4/kernel/hca/cq.c	(working copy)
@@ -98,7 +98,7 @@
 	// return the result
 	*p_size = p_ib_cq->cqe;
 
-	if (ph_cq) *ph_cq = (ib_cq_handle_t)p_ib_cq;
+	*ph_cq = (ib_cq_handle_t)p_ib_cq;
 
 	status = IB_SUCCESS;
 	
Index: hw/mlx4/kernel/hca/data.c
===================================================================
--- hw/mlx4/kernel/hca/data.c	(revision 3414)
+++ hw/mlx4/kernel/hca/data.c	(working copy)
@@ -430,7 +430,6 @@
 		err = p_ib_dev->x.find_cached_gid((struct ib_device *)p_ib_dev, 
 			(union ib_gid	*)p_ib_av_attr->grh.src_gid.raw, &port_num, &gid_index);
 		if (err) {
-
 			HCA_PRINT(TRACE_LEVEL_ERROR ,HCA_DBG_SHIM ,
 			("ib_find_cached_gid failed %d (%#x). Using default:  sgid_index = 0\n", err, err));
 			gid_index = 0;
@@ -482,7 +481,6 @@
 			p_ib_ah_attr->port_num, p_ib_ah_attr->grh.sgid_index,
 			(union ib_gid*)p_ib_av_attr->grh.src_gid.raw );
 		if (err) {
-
 			HCA_PRINT(TRACE_LEVEL_ERROR ,HCA_DBG_SHIM ,
 			("ib_get_cached_gid failed %d (%#x). Using default:  sgid_index = 0\n", err, err));
 		}
Index: hw/mlx4/kernel/hca/hverbs.c
===================================================================
--- hw/mlx4/kernel/hca/hverbs.c	(revision 3414)
+++ hw/mlx4/kernel/hca/hverbs.c	(working copy)
@@ -69,11 +69,13 @@
 {
 	struct ib_mr *mr;
 
-	if ( pd->device->reg_phys_mr )
+	if ( pd->device->reg_phys_mr ) {
 		mr = pd->device->reg_phys_mr(pd, phys_buf_array, num_phys_buf,
 			mr_access_flags, iova_start);
-	else
+    }
+	else {
 		mr = ERR_PTR(-ENOSYS);
+    }
 
 	if (!IS_ERR(mr)) {
 		mr->device  = pd->device;
@@ -218,11 +220,12 @@
 		p_req = (struct ibv_create_cq*)(ULONG_PTR)p_umv_buf->p_inout_buf;
 		p_resp = (struct ibv_create_cq_resp*)(ULONG_PTR)
 			p_umv_buf->p_inout_buf;
-		INIT_UDATA(&udata, &p_req->buf_addr, &p_resp->cqn, 
-			sizeof(struct mlx4_ib_create_cq), sizeof(struct mlx4_ib_create_cq_resp));
+		INIT_UDATA(&udata, p_req, p_resp, 
+			sizeof(struct ibv_create_cq), sizeof(struct ibv_create_cq_resp));
 	}
-	else 
+	else {
 		p_udata = NULL;
+    }
 
 	// create cq
 	cq = p_ibdev->create_cq(p_ibdev, cqe, 0, p_uctx, p_udata);
@@ -276,8 +279,8 @@
 		p_req = (struct ibv_create_cq*)(ULONG_PTR)p_umv_buf->p_inout_buf;
 		p_resp = (struct ibv_create_cq_resp*)(ULONG_PTR)
 			p_umv_buf->p_inout_buf;
-		INIT_UDATA(&udata, &p_req->buf_addr, &p_resp->cqn, 
-			sizeof(struct mlx4_ib_create_cq), sizeof(struct mlx4_ib_create_cq_resp));
+		INIT_UDATA(&udata, p_req, p_resp, 
+			sizeof(struct ibv_create_cq), sizeof(struct ibv_create_cq_resp));
 	}
 	else 
 		p_udata = NULL;
@@ -369,11 +372,12 @@
 		// prepare user parameters
 		p_req = (struct ibv_create_qp*)(ULONG_PTR)p_umv_buf->p_inout_buf;
 		p_resp = (struct ibv_create_qp_resp*)(ULONG_PTR)p_umv_buf->p_inout_buf;
-		INIT_UDATA(&udata, &p_req->buf_addr, NULL, 
-			sizeof(struct mlx4_ib_create_qp), 0);
+		INIT_UDATA(&udata, p_req, NULL, 
+			sizeof(struct ibv_create_qp), 0);
 	}
-	else 
+	else {
 		p_udata = NULL;
+    }
 
 	p_ib_qp = pd->device->create_qp( pd, qp_init_attr, p_udata );
 
@@ -485,11 +489,12 @@
 		// prepare user parameters
 		p_req = (struct ibv_create_srq*)(ULONG_PTR)p_umv_buf->p_inout_buf;
 		p_resp = (struct ibv_create_srq_resp*)(ULONG_PTR)p_umv_buf->p_inout_buf;
-		INIT_UDATA(&udata, &p_req->buf_addr, &p_resp->srqn, 
+		INIT_UDATA(&udata, p_req, p_resp, 
 			sizeof(struct ibv_create_srq), sizeof(struct ibv_create_srq_resp));
 	}
-	else 
+	else {
 		p_udata = NULL;
+    }
 
 	p_ib_srq = pd->device->create_srq( pd, srq_init_attr, p_udata );
 	if (IS_ERR(p_ib_srq)) {
Index: hw/mlx4/kernel/hca/qp.c
===================================================================
--- hw/mlx4/kernel/hca/qp.c	(revision 3414)
+++ hw/mlx4/kernel/hca/qp.c	(working copy)
@@ -122,21 +122,12 @@
 	qp_init_attr.send_cq = (struct ib_cq *)p_create_attr->h_sq_cq;
 	qp_init_attr.recv_cq = (struct ib_cq *)p_create_attr->h_rq_cq;
 	qp_init_attr.srq = (struct ib_srq *)p_create_attr->h_srq;
-	if( p_umv_buf && p_umv_buf->command ) {
-		qp_init_attr.cap.max_recv_sge = p_req->max_recv_sge;
-		qp_init_attr.cap.max_send_sge = p_req->max_send_sge;
-		qp_init_attr.cap.max_recv_wr = p_req->max_recv_wr;
-		qp_init_attr.cap.max_send_wr = p_req->max_send_wr;
-		qp_init_attr.cap.max_inline_data = p_req->max_inline_data;
-	}
-	else {
 		qp_init_attr.cap.max_recv_sge = p_create_attr->rq_sge;
 		qp_init_attr.cap.max_send_sge = p_create_attr->sq_sge;
 		qp_init_attr.cap.max_recv_wr = p_create_attr->rq_depth;
 		qp_init_attr.cap.max_send_wr = p_create_attr->sq_depth;
 		qp_init_attr.cap.max_inline_data = p_create_attr->sq_max_inline;
 		
-	}
 	qp_init_attr.sq_sig_type = (p_create_attr->sq_signaled) ? IB_SIGNAL_ALL_WR : IB_SIGNAL_REQ_WR;
 	qp_init_attr.qp_type = to_qp_type(p_create_attr->qp_type);
 	qp_init_attr.port_num = port_num;
@@ -167,7 +158,7 @@
 	}
 	
 	// return the results
-	if (ph_qp) *ph_qp = (ib_qp_handle_t)p_ib_qp;
+	*ph_qp = (ib_qp_handle_t)p_ib_qp;
 
 	status = IB_SUCCESS;
 	goto end;
@@ -363,7 +354,6 @@
 	ci_umv_buf_t umv_buf;
 	ib_api_status_t status;
 	struct ibv_modify_qp_resp resp;
-	void *buf = &resp;
 
 	HCA_ENTER(HCA_DBG_QP);
 
@@ -375,8 +365,8 @@
 	/* imitate umv_buf */
 	umv_buf.command = TRUE;	/* special case for NDI. Usually it's TRUE */
 	umv_buf.input_size = 0;
-	umv_buf.output_size = sizeof(struct ibv_modify_qp_resp);
-	umv_buf.p_inout_buf = (ULONG_PTR)buf;
+	umv_buf.output_size = sizeof(resp);
+	umv_buf.p_inout_buf = (ULONG_PTR)&resp;
 
 	status = mlnx_modify_qp ( h_qp, p_modify_attr, p_qp_attr, &umv_buf );
 
Index: hw/mlx4/kernel/bus/inc/ib_verbs.h
===================================================================
--- hw/mlx4/kernel/bus/inc/ib_verbs.h	(revision 3414)
+++ hw/mlx4/kernel/bus/inc/ib_verbs.h	(working copy)
@@ -50,39 +50,9 @@
 };
 
 #include "ib_verbs_ex.h"
+#include <ws2def.h>
+#include <ws2ipdef.h>
 
-/*
- *	IPv6 address structure
- */
-
-struct ib_in6_addr
-{
-	union 
-	{
-		__u8		u6_addr8[16];
-		__be16		u6_addr16[8];
-		__be32		u6_addr32[4];
-	} in6_u;
-#ifndef	s6_addr
-#define s6_addr			in6_u.u6_addr8
-#endif
-#define s6_addr16		in6_u.u6_addr16
-#define s6_addr32		in6_u.u6_addr32
-};
-
-
-struct ib_sockaddr_in6 {
-	unsigned short int	sin6_family;    /* AF_INET6 */
-	__be16			sin6_port;      /* Transport layer port # */
-	__be32			sin6_flowinfo;  /* IPv6 flow information */
-	struct ib_in6_addr		sin6_addr;      /* IPv6 address */
-	__u32			sin6_scope_id;  /* scope id (new in RFC2553) */
-};
-
-#ifndef	AF_INET6
-#define AF_INET6	10	/* IP version 6			*/
-#endif
-
 enum rdma_node_type {
 	/* IB values map to NodeInfo:NodeType. */
 	RDMA_NODE_IB_CA 	= 1,
Index: hw/mlx4/kernel/bus/ib/mlx4_ib.h
===================================================================
--- hw/mlx4/kernel/bus/ib/mlx4_ib.h	(revision 3414)
+++ hw/mlx4/kernel/bus/ib/mlx4_ib.h	(working copy)
@@ -42,7 +42,16 @@
 #include "device.h"
 #include "doorbell.h"
 #include "ib_pack.h"
+
+
+inline void rdma_get_ll_mac(const union ib_gid *gid, u8 *mac)
+{
+	memcpy(mac, &gid->raw[8], 3);
+	memcpy(mac + 3, &gid->raw[13], 3);
+	mac[0] ^= 2;   
+}
 
+
 enum {
 	MLX4_IB_DB_PER_PAGE	= PAGE_SIZE / 4
 };
Index: hw/mlx4/kernel/bus/ib/qp.c
===================================================================
--- hw/mlx4/kernel/bus/ib/qp.c	(revision 3414)
+++ hw/mlx4/kernel/bus/ib/qp.c	(working copy)
@@ -92,8 +92,6 @@
 
 //?????????????????	IB_WR_RDMA_READ_WITH_INV,  //???????????????
 
-extern inline void rdma_get_ll_mac(struct ib_in6_addr *addr, u8 *mac);
-
 static struct mlx4_ib_sqp *to_msqp(struct mlx4_ib_qp *mqp)
 {
 	return container_of(mqp, struct mlx4_ib_sqp, qp);
@@ -1733,7 +1731,7 @@
 		memcpy(eth->eth.dmac_h, ah->av.eth.mac_0_1, 2);
 		memcpy(eth->eth.dmac_h + 2, ah->av.eth.mac_2_5, 2);
 		memcpy(eth->eth.dmac_l, ah->av.eth.mac_2_5 + 2, 2);
-		rdma_get_ll_mac((struct ib_in6_addr *)&grh->source_gid, mac);
+		rdma_get_ll_mac(&grh->source_gid, mac);
 
 		tmp = mac;
 		memcpy(eth->eth.smac_h, tmp, 2);

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 12.hca_cleanup.patch
Type: application/octet-stream
Size: 10879 bytes
Desc: 12.hca_cleanup.patch
URL: <http://lists.openfabrics.org/pipermail/ofw/attachments/20120619/b4b4f9c7/attachment.obj>


More information about the ofw mailing list