[openib-general] [PATCH] process_mad extension

Michael S. Tsirkin mst at mellanox.co.il
Mon Jan 10 07:18:07 PST 2005


Hello!
Quoting r. Roland Dreier (roland at topspin.com) "Re: [openib-general] Re: RFC: process_mad extension":
> Thanks, but:
> 
>  - The patch has some bad spacing in places like:
> 
> +	wc->wr_id=send_wr->wr_id;
> +	wc->status=IB_WC_SUCCESS;
> 
>    as well as C++-style declarations:
> 
> +		      struct ib_wc* in_wc,
> +		      struct ib_grh* in_grh,
> 
>  - Rather than changing IB_MAD_IGNORE_MKEY to IB_MAD_IGNORE_MKEY_BKEY,
>    why not introduce new flags IB_MAD_IGNORE_BKEY and IB_MAD_IGNORE_ALL
>    with IB_MAD_IGNORE_ALL set to IB_MAD_IGNORE_MKEY | IB_MAD_IGNORE_BKEY?
>    This matches the behavior of the FW better and lets existing code
>    such as sysfs.c remain unchanged.
> 
>  - This last comment is just taste and maybe I'm wrong, but I don't
>    see much advantage to defining a separate function build_smp_wc()
>    that is only called from one place.
> 
> Thanks,
>   Roland


Ok, I addressed all of these except the "C++-style" comment
which I dont understand. Roland - how are these parameters
in my code different from the next line where I see struct mad* in_mad?
What is C++ about them?

I got rid of some lines of code by reusing the build_smp_wc
function, so that shall be fine too now.

While I was at it, I saw what seems like a small bug: when local_completions
generates a wc, the port number field was uninitialised.

After the patch the wc is now built in the build_smp_wc function,
everything will be now initialised, so that will be fixed.


Patch attached, I think it shall be fine to go in now. Pls apply.

Signed-off-by: Michael S. Tsirkin <mst at mellanox.co.il>

Index: include/ib_verbs.h
===================================================================
--- include/ib_verbs.h	(revision 1498)
+++ include/ib_verbs.h	(working copy)
@@ -684,9 +684,12 @@ struct ib_fmr {
 };
 
 struct ib_mad;
+struct ib_grh;
 
 enum ib_process_mad_flags {
-	IB_MAD_IGNORE_MKEY	= 1
+	IB_MAD_IGNORE_MKEY	= 1,
+	IB_MAD_IGNORE_BKEY	= 2,
+	IB_MAD_IGNORE_ALL	= 3
 };
 
 enum ib_mad_result {
@@ -812,7 +815,8 @@ struct ib_device {
 	int                        (*process_mad)(struct ib_device *device,
 						  int process_mad_flags,
 						  u8 port_num,
-						  u16 source_lid,
+						  struct ib_wc* in_wc,
+						  struct ib_grh* in_grh,
 						  struct ib_mad *in_mad,
 						  struct ib_mad *out_mad);
 
Index: core/mad.c
===================================================================
--- core/mad.c	(revision 1498)
+++ core/mad.c	(working copy)
@@ -614,6 +614,23 @@ static void snoop_recv(struct ib_mad_qp_
 	spin_unlock_irqrestore(&qp_info->snoop_lock, flags);
 }
 
+static void build_smp_wc(u64 wr_id, u16 slid, u16 pkey_index, u8 port_num,
+		struct ib_wc* wc)
+{
+	memset(wc,0,sizeof *wc);
+	wc->wr_id = wr_id;
+	wc->status = IB_WC_SUCCESS;
+	wc->opcode = IB_WC_RECV;
+	wc->pkey_index = pkey_index;
+	wc->byte_len = sizeof(struct ib_mad) + sizeof(struct ib_grh);
+	wc->src_qp = IB_QP0;
+	wc->qp_num = IB_QP0;
+	wc->slid = slid;
+	wc->sl = 0;
+	wc->dlid_path_bits = 0;
+	wc->port_num = port_num;
+}
+
 /*
  * Return 0 if SMP is to be sent
  * Return 1 if SMP was consumed locally (whether or not solicited)
@@ -629,6 +646,7 @@ static int handle_outgoing_dr_smp(struct
 	struct ib_mad_private *mad_priv;
 	struct ib_device *device = mad_agent_priv->agent.device;
 	u8 port_num = mad_agent_priv->agent.port_num;
+	struct ib_wc mad_wc;
 
 	if (!smi_handle_dr_smp_send(smp, device->node_type, port_num)) {
 		ret = -EINVAL;
@@ -658,7 +676,12 @@ static int handle_outgoing_dr_smp(struct
 		kfree(local);
 		goto out;
 	}
-	ret = device->process_mad(device, 0, port_num, smp->dr_slid,
+	
+	build_smp_wc(send_wr->wr_id, smp->dr_slid, send_wr->wr.ud.pkey_index,
+		     send_wr->wr.ud.port_num, &mad_wc);
+
+	/* No grh for dr smp. */
+	ret = device->process_mad(device, 0, port_num, &mad_wc, NULL,
 				  (struct ib_mad *)smp,
 				  (struct ib_mad *)&mad_priv->mad);
 	switch (ret)
@@ -1594,7 +1617,7 @@ local:
 
 		ret = port_priv->device->process_mad(port_priv->device, 0,
 						     port_priv->port_num,
-						     wc->slid,
+						     wc, &recv->grh,
 						     &recv->mad.mad,
 						     &response->mad.mad);
 		if (ret & IB_MAD_RESULT_SUCCESS) {
@@ -2014,19 +2037,10 @@ static void local_completions(void *data
 			 * Defined behavior is to complete response
 			 * before request
 			 */
-			wc.wr_id = local->wr_id;
-			wc.status = IB_WC_SUCCESS;
-			wc.opcode = IB_WC_RECV;
-			wc.vendor_err = 0;
-			wc.byte_len = sizeof(struct ib_mad) +
-				      sizeof(struct ib_grh);
-			wc.src_qp = IB_QP0;
-			wc.wc_flags = 0;	/* No GRH */
-			wc.pkey_index = 0;
-			wc.slid = IB_LID_PERMISSIVE;
-			wc.sl = 0;
-			wc.dlid_path_bits = 0;
-			wc.qp_num = IB_QP0;
+			build_smp_wc(local->wr_id, IB_LID_PERMISSIVE,
+				     0 /* pkey index */,
+				     mad_agent_priv->agent.port_num, &wc);
+
 			local->mad_priv->header.recv_wc.wc = &wc;
 			local->mad_priv->header.recv_wc.mad_len =
 						sizeof(struct ib_mad);
Index: core/sysfs.c
===================================================================
--- core/sysfs.c	(revision 1498)
+++ core/sysfs.c	(working copy)
@@ -315,8 +315,8 @@ static ssize_t show_pma_counter(struct i
 
 	in_mad->data[41] = p->port_num;	/* PortSelect field */
 
-	if ((p->ibdev->process_mad(p->ibdev, IB_MAD_IGNORE_MKEY, p->port_num, 0xffff,
-				   in_mad, out_mad) &
+	if ((p->ibdev->process_mad(p->ibdev, IB_MAD_IGNORE_MKEY,
+		 p->port_num, NULL, NULL, in_mad, out_mad) &
 	     (IB_MAD_RESULT_SUCCESS | IB_MAD_RESULT_REPLY)) !=
 	    (IB_MAD_RESULT_SUCCESS | IB_MAD_RESULT_REPLY)) {
 		ret = -EINVAL;
Index: hw/mthca/mthca_dev.h
===================================================================
--- hw/mthca/mthca_dev.h	(revision 1498)
+++ hw/mthca/mthca_dev.h	(working copy)
@@ -381,7 +381,8 @@ int mthca_multicast_detach(struct ib_qp 
 int mthca_process_mad(struct ib_device *ibdev,
 		      int mad_flags,
 		      u8 port_num,
-		      u16 slid,
+		      struct ib_wc* in_wc,
+		      struct ib_grh* in_grh,
 		      struct ib_mad *in_mad,
 		      struct ib_mad *out_mad);
 int mthca_create_agents(struct mthca_dev *dev);
Index: hw/mthca/mthca_mad.c
===================================================================
--- hw/mthca/mthca_mad.c	(revision 1498)
+++ hw/mthca/mthca_mad.c	(working copy)
@@ -185,12 +185,14 @@ static void forward_trap(struct mthca_de
 int mthca_process_mad(struct ib_device *ibdev,
 		      int mad_flags,
 		      u8 port_num,
-		      u16 slid,
+		      struct ib_wc* in_wc,
+		      struct ib_grh* in_grh,
 		      struct ib_mad *in_mad,
 		      struct ib_mad *out_mad)
 {
 	int err;
 	u8 status;
+	u16 slid = in_wc ? in_wc->slid : IB_LID_PERMISSIVE;
 
 	/* Forward locally generated traps to the SM */
 	if (in_mad->mad_hdr.method == IB_MGMT_METHOD_TRAP &&



More information about the general mailing list