[ofa-general] [PATCHv4 03/10] ib_core: RDMAoE support only QP1

[Sean Hefty]

>diff --git a/drivers/infiniband/core/mad.c b/drivers/infiniband/core/mad.c
>index 7b737c4..de83c71 100644
>--- a/drivers/infiniband/core/mad.c
>+++ b/drivers/infiniband/core/mad.c
>@@ -199,6 +199,16 @@ struct ib_mad_agent *ib_register_mad_agent(struct
>ib_device *device,
> 	unsigned long flags;
> 	u8 mgmt_class, vclass;
>
>+	/* Validate device and port */
>+	port_priv = ib_get_mad_port(device, port_num);
>+	if (!port_priv) {
>+		ret = ERR_PTR(-ENODEV);
>+		goto error1;
>+	}
>+
>+	if (!port_priv->qp_info[qp_type].qp)
>+		return NULL;

It seems odd that the first if has 'goto error1', but the second if simply
returns NULL. 

>+
> 	/* Validate parameters */
> 	qpn = get_spl_qp_index(qp_type);
> 	if (qpn == -1)
>@@ -260,13 +270,6 @@ struct ib_mad_agent *ib_register_mad_agent(struct
>ib_device *device,
> 			goto error1;
> 	}
>
>-	/* Validate device and port */
>-	port_priv = ib_get_mad_port(device, port_num);
>-	if (!port_priv) {
>-		ret = ERR_PTR(-ENODEV);
>-		goto error1;
>-	}
>-
> 	/* Allocate structures */
> 	mad_agent_priv = kzalloc(sizeof *mad_agent_priv, GFP_KERNEL);
> 	if (!mad_agent_priv) {
>@@ -556,6 +559,9 @@ int ib_unregister_mad_agent(struct ib_mad_agent *mad_agent)
> 	struct ib_mad_agent_private *mad_agent_priv;
> 	struct ib_mad_snoop_private *mad_snoop_priv;
>
>+	if (!mad_agent)
>+		return 0;

Why would a kernel client call ib_unregister_mad_agent with a NULL pointer?

>+
> 	/* If the TID is zero, the agent can only snoop. */
> 	if (mad_agent->hi_tid) {
> 		mad_agent_priv = container_of(mad_agent,
>@@ -2602,6 +2608,9 @@ static void cleanup_recv_queue(struct ib_mad_qp_info
>*qp_info)
> 	struct ib_mad_private *recv;
> 	struct ib_mad_list_head *mad_list;
>
>+	if (!qp_info->qp)
>+		return;
>+
> 	while (!list_empty(&qp_info->recv_queue.list)) {
>
> 		mad_list = list_entry(qp_info->recv_queue.list.next,
>@@ -2643,6 +2652,9 @@ static int ib_mad_port_start(struct ib_mad_port_private
>*port_priv)
>
> 	for (i = 0; i < IB_MAD_QPS_CORE; i++) {
> 		qp = port_priv->qp_info[i].qp;
>+		if (!qp)
>+			continue;
>+
> 		/*
> 		 * PKey index for QP1 is irrelevant but
> 		 * one is needed for the Reset to Init transition
>@@ -2684,6 +2696,9 @@ static int ib_mad_port_start(struct ib_mad_port_private
>*port_priv)
> 	}
>
> 	for (i = 0; i < IB_MAD_QPS_CORE; i++) {
>+		if (!port_priv->qp_info[i].qp)
>+			continue;
>+
> 		ret = ib_mad_post_receive_mads(&port_priv->qp_info[i], NULL);
> 		if (ret) {
> 			printk(KERN_ERR PFX "Couldn't post receive WRs\n");
>@@ -2762,6 +2777,9 @@ error:
>
> static void destroy_mad_qp(struct ib_mad_qp_info *qp_info)
> {
>+	if (!qp_info->qp)
>+		return;
>+
> 	ib_destroy_qp(qp_info->qp);
> 	kfree(qp_info->snoop_table);
> }
>@@ -2777,6 +2795,7 @@ static int ib_mad_port_open(struct ib_device *device,
> 	struct ib_mad_port_private *port_priv;
> 	unsigned long flags;
> 	char name[sizeof "ib_mad123"];
>+	int has_smi;
>
> 	/* Create new device info */
> 	port_priv = kzalloc(sizeof *port_priv, GFP_KERNEL);
>@@ -2793,6 +2812,10 @@ static int ib_mad_port_open(struct ib_device *device,
> 	init_mad_qp(port_priv, &port_priv->qp_info[1]);
>
> 	cq_size = (IB_MAD_QP_SEND_SIZE + IB_MAD_QP_RECV_SIZE) * 2;
>+	has_smi = rdma_port_get_transport(device, port_num) ==
RDMA_TRANSPORT_IB;
>+	if (has_smi)
>+		cq_size *= 2;

cq_size is doubled twice

I really wish there were a cleaner way to add this support that didn't involve
adding so many checks throughout the code.  It's hard to know if checks were
added in all the places that were needed.  I can't think of a clever way to
handle QP 0.

- Sean