[infiniband] Respect hop pointer when building directed route SMP return path

The return path in directed route SMPs lists the egress ports in order
from SM to node, rather than from node to SM.

To write to the correct offset within the return path, we need to
parse the hop pointer.  This is held within the class-specific data
portion of the MAD header, which was previously unused by us and
defined to be a uint16_t.  Define this field to be a union type; this
requires some rearrangement of ib_mad.h and corresponding changes to
ipoib.c.

src/drivers/infiniband/ib_sma.c

@@ -264,6 +264,8 @@ static int ib_sma_mad ( struct ib_sma *sma, union ib_mad *mad ) {
 	struct ib_mad_hdr *hdr = &mad->hdr;
 	struct ib_mad_smp *smp = &mad->smp;
 	struct ib_sma_handler *handler = NULL;
+	unsigned int hop_pointer;
+	unsigned int hop_count;
 	int rc;
 
 	DBGC ( sma, "SMA %p received SMP with bv=%02x mc=%02x cv=%02x "
@@ -336,7 +338,17 @@ static int ib_sma_mad ( struct ib_sma *sma, union ib_mad *mad ) {
 	/* Set response fields for directed route SMPs */
 	if ( hdr->mgmt_class == IB_MGMT_CLASS_SUBN_DIRECTED_ROUTE ) {
 		hdr->status |= htons ( IB_SMP_STATUS_D_INBOUND );
-		smp->return_path.hops[0] = ibdev->port;
+		hop_pointer = smp->mad_hdr.class_specific.smp.hop_pointer;
+		hop_count = smp->mad_hdr.class_specific.smp.hop_count;
+		assert ( hop_count == hop_pointer );
+		if ( hop_pointer < ( sizeof ( smp->return_path.hops ) /
+				     sizeof ( smp->return_path.hops[0] ) ) ) {
+			smp->return_path.hops[hop_pointer] = ibdev->port;
+		} else {
+			DBGC ( sma, "SMA %p invalid hop pointer %d\n",
+			       sma, hop_pointer );
+			return -EINVAL;
+		}
 	}
 
 	DBGC ( sma, "SMA %p responding with status=%04x\n",

src/drivers/net/ipoib.c

@@ -440,31 +440,32 @@ static int ipoib_get_path_record ( struct ipoib_device *ipoib,
 				   struct ib_gid *gid ) {
 	struct ib_device *ibdev = ipoib->ibdev;
 	struct io_buffer *iobuf;
-	struct ib_mad_path_record *path_record;
+	struct ib_mad_sa *sa;
 	struct ib_address_vector av;
 	int rc;
 
 	/* Allocate I/O buffer */
-	iobuf = alloc_iob ( sizeof ( *path_record ) );
+	iobuf = alloc_iob ( sizeof ( *sa ) );
 	if ( ! iobuf )
 		return -ENOMEM;
-	iob_put ( iobuf, sizeof ( *path_record ) );
-	path_record = iobuf->data;
-	memset ( path_record, 0, sizeof ( *path_record ) );
+	iob_put ( iobuf, sizeof ( *sa ) );
+	sa = iobuf->data;
+	memset ( sa, 0, sizeof ( *sa ) );
 
 	/* Construct path record request */
-	path_record->mad_hdr.base_version = IB_MGMT_BASE_VERSION;
-	path_record->mad_hdr.mgmt_class = IB_MGMT_CLASS_SUBN_ADM;
-	path_record->mad_hdr.class_version = 2;
-	path_record->mad_hdr.method = IB_MGMT_METHOD_GET;
-	path_record->mad_hdr.attr_id = htons ( IB_SA_ATTR_PATH_REC );
-	path_record->mad_hdr.tid[0] = IPOIB_TID_GET_PATH_REC;
-	path_record->mad_hdr.tid[1] = ipoib_meta_tid++;
-	path_record->sa_hdr.comp_mask[1] =
+	sa->mad_hdr.base_version = IB_MGMT_BASE_VERSION;
+	sa->mad_hdr.mgmt_class = IB_MGMT_CLASS_SUBN_ADM;
+	sa->mad_hdr.class_version = 2;
+	sa->mad_hdr.method = IB_MGMT_METHOD_GET;
+	sa->mad_hdr.attr_id = htons ( IB_SA_ATTR_PATH_REC );
+	sa->mad_hdr.tid[0] = IPOIB_TID_GET_PATH_REC;
+	sa->mad_hdr.tid[1] = ipoib_meta_tid++;
+	sa->sa_hdr.comp_mask[1] =
 		htonl ( IB_SA_PATH_REC_DGID | IB_SA_PATH_REC_SGID );
-	memcpy ( &path_record->dgid, gid, sizeof ( path_record->dgid ) );
-	memcpy ( &path_record->sgid, &ibdev->gid,
-		 sizeof ( path_record->sgid ) );
+	memcpy ( &sa->sa_data.path_record.dgid, gid,
+		 sizeof ( sa->sa_data.path_record.dgid ) );
+	memcpy ( &sa->sa_data.path_record.sgid, &ibdev->gid,
+		 sizeof ( sa->sa_data.path_record.sgid ) );
 
 	/* Construct address vector */
 	memset ( &av, 0, sizeof ( av ) );
@@ -497,35 +498,35 @@ static int ipoib_mc_member_record ( struct ipoib_device *ipoib,
 				    struct ib_gid *gid, int join ) {
 	struct ib_device *ibdev = ipoib->ibdev;
 	struct io_buffer *iobuf;
-	struct ib_mad_mc_member_record *mc_member_record;
+	struct ib_mad_sa *sa;
 	struct ib_address_vector av;
 	int rc;
 
 	/* Allocate I/O buffer */
-	iobuf = alloc_iob ( sizeof ( *mc_member_record ) );
+	iobuf = alloc_iob ( sizeof ( *sa ) );
 	if ( ! iobuf )
 		return -ENOMEM;
-	iob_put ( iobuf, sizeof ( *mc_member_record ) );
-	mc_member_record = iobuf->data;
-	memset ( mc_member_record, 0, sizeof ( *mc_member_record ) );
+	iob_put ( iobuf, sizeof ( *sa ) );
+	sa = iobuf->data;
+	memset ( sa, 0, sizeof ( *sa ) );
 
 	/* Construct path record request */
-	mc_member_record->mad_hdr.base_version = IB_MGMT_BASE_VERSION;
-	mc_member_record->mad_hdr.mgmt_class = IB_MGMT_CLASS_SUBN_ADM;
-	mc_member_record->mad_hdr.class_version = 2;
-	mc_member_record->mad_hdr.method = 
+	sa->mad_hdr.base_version = IB_MGMT_BASE_VERSION;
+	sa->mad_hdr.mgmt_class = IB_MGMT_CLASS_SUBN_ADM;
+	sa->mad_hdr.class_version = 2;
+	sa->mad_hdr.method =
 		( join ? IB_MGMT_METHOD_SET : IB_MGMT_METHOD_DELETE );
-	mc_member_record->mad_hdr.attr_id = htons ( IB_SA_ATTR_MC_MEMBER_REC );
-	mc_member_record->mad_hdr.tid[0] = IPOIB_TID_MC_MEMBER_REC;
-	mc_member_record->mad_hdr.tid[1] = ipoib_meta_tid++;
-	mc_member_record->sa_hdr.comp_mask[1] =
+	sa->mad_hdr.attr_id = htons ( IB_SA_ATTR_MC_MEMBER_REC );
+	sa->mad_hdr.tid[0] = IPOIB_TID_MC_MEMBER_REC;
+	sa->mad_hdr.tid[1] = ipoib_meta_tid++;
+	sa->sa_hdr.comp_mask[1] =
 		htonl ( IB_SA_MCMEMBER_REC_MGID | IB_SA_MCMEMBER_REC_PORT_GID |
 			IB_SA_MCMEMBER_REC_JOIN_STATE );
-	mc_member_record->scope__join_state = 1;
-	memcpy ( &mc_member_record->mgid, gid,
-		 sizeof ( mc_member_record->mgid ) );
-	memcpy ( &mc_member_record->port_gid, &ibdev->gid,
-		 sizeof ( mc_member_record->port_gid ) );
+	sa->sa_data.mc_member_record.scope__join_state = 1;
+	memcpy ( &sa->sa_data.mc_member_record.mgid, gid,
+		 sizeof ( sa->sa_data.mc_member_record.mgid ) );
+	memcpy ( &sa->sa_data.mc_member_record.port_gid, &ibdev->gid,
+		 sizeof ( sa->sa_data.mc_member_record.port_gid ) );
 
 	/* Construct address vector */
 	memset ( &av, 0, sizeof ( av ) );
@@ -701,7 +702,7 @@ static void ipoib_meta_complete_send ( struct ib_device *ibdev __unused,
  * @v path_record	Path record
  */
 static void ipoib_recv_path_record ( struct ipoib_device *ipoib,
-				     struct ib_mad_path_record *path_record ) {
+				     struct ib_path_record *path_record ) {
 	struct ipoib_peer *peer;
 
 	/* Locate peer cache entry */
@@ -728,7 +729,7 @@ static void ipoib_recv_path_record ( struct ipoib_device *ipoib,
  * @v mc_member_record	Multicast membership record
  */
 static void ipoib_recv_mc_member_record ( struct ipoib_device *ipoib,
-			  struct ib_mad_mc_member_record *mc_member_record ) {
+			       struct ib_mc_member_record *mc_member_record ) {
 	int joined;
 	int rc;
 
@@ -765,7 +766,7 @@ ipoib_meta_complete_recv ( struct ib_device *ibdev __unused,
 			   struct io_buffer *iobuf, int rc ) {
 	struct net_device *netdev = ib_qp_get_ownerdata ( qp );
 	struct ipoib_device *ipoib = netdev->priv;
-	union ib_mad *mad;
+	struct ib_mad_sa *sa;
 
 	if ( rc != 0 ) {
 		DBGC ( ipoib, "IPoIB %p metadata RX completion error: %s\n",
@@ -773,31 +774,32 @@ ipoib_meta_complete_recv ( struct ib_device *ibdev __unused,
 		goto done;
 	}
 
-	if ( iob_len ( iobuf ) < sizeof ( *mad ) ) {
+	if ( iob_len ( iobuf ) < sizeof ( *sa ) ) {
 		DBGC ( ipoib, "IPoIB %p received metadata packet too short "
 		       "to contain reply\n", ipoib );
 		DBGC_HD ( ipoib, iobuf->data, iob_len ( iobuf ) );
 		goto done;
 	}
-	mad = iobuf->data;
+	sa = iobuf->data;
 
-	if ( mad->hdr.status != 0 ) {
+	if ( sa->mad_hdr.status != 0 ) {
 		DBGC ( ipoib, "IPoIB %p metadata RX err status %04x\n",
-		       ipoib, ntohs ( mad->hdr.status ) );
+		       ipoib, ntohs ( sa->mad_hdr.status ) );
 		goto done;
 	}
 
-	switch ( mad->hdr.tid[0] ) {
+	switch ( sa->mad_hdr.tid[0] ) {
 	case IPOIB_TID_GET_PATH_REC:
-		ipoib_recv_path_record ( ipoib, &mad->path_record );
+		ipoib_recv_path_record ( ipoib, &sa->sa_data.path_record );
 		break;
 	case IPOIB_TID_MC_MEMBER_REC:
-		ipoib_recv_mc_member_record ( ipoib, &mad->mc_member_record );
+		ipoib_recv_mc_member_record ( ipoib,
+					      &sa->sa_data.mc_member_record );
 		break;
 	default:
 		DBGC ( ipoib, "IPoIB %p unwanted response:\n",
 		       ipoib );
-		DBGC_HD ( ipoib, mad, sizeof ( *mad ) );
+		DBGC_HD ( ipoib, sa, sizeof ( *sa ) );
 		break;
 	}
 

src/include/gpxe/ib_mad.h

@@ -10,83 +10,24 @@
 #include <stdint.h>
 #include <gpxe/ib_packet.h>
 
-/** A management datagram common header
+/*****************************************************************************
  *
- * Defined in section 13.4.2 of the IBA.
- */
-struct ib_mad_hdr {
-	uint8_t base_version;
-	uint8_t mgmt_class;
-	uint8_t class_version;
-	uint8_t method;
-	uint16_t status;
-	uint16_t class_specific;
-	uint32_t tid[2];
-	uint16_t attr_id;
-	uint8_t reserved[2];
-	uint32_t attr_mod;
-} __attribute__ (( packed ));
-
-/* Management base version */
-#define IB_MGMT_BASE_VERSION			1
-
-/* Management classes */
-#define IB_MGMT_CLASS_SUBN_LID_ROUTED		0x01
-#define IB_MGMT_CLASS_SUBN_DIRECTED_ROUTE	0x81
-#define IB_MGMT_CLASS_SUBN_ADM			0x03
-#define IB_MGMT_CLASS_PERF_MGMT			0x04
-#define IB_MGMT_CLASS_BM			0x05
-#define IB_MGMT_CLASS_DEVICE_MGMT		0x06
-#define IB_MGMT_CLASS_CM			0x07
-#define IB_MGMT_CLASS_SNMP			0x08
-#define IB_MGMT_CLASS_VENDOR_RANGE2_START	0x30
-#define IB_MGMT_CLASS_VENDOR_RANGE2_END		0x4F
-
-/* Management methods */
-#define IB_MGMT_METHOD_GET			0x01
-#define IB_MGMT_METHOD_SET			0x02
-#define IB_MGMT_METHOD_GET_RESP			0x81
-#define IB_MGMT_METHOD_SEND			0x03
-#define IB_MGMT_METHOD_TRAP			0x05
-#define IB_MGMT_METHOD_REPORT			0x06
-#define IB_MGMT_METHOD_REPORT_RESP		0x86
-#define IB_MGMT_METHOD_TRAP_REPRESS		0x07
-#define IB_MGMT_METHOD_DELETE			0x15
-
-/* Status codes */
-#define IB_MGMT_STATUS_OK			0x0000
-#define IB_MGMT_STATUS_BAD_VERSION		0x0001
-#define IB_MGMT_STATUS_UNSUPPORTED_METHOD	0x0002
-#define IB_MGMT_STATUS_UNSUPPORTED_METHOD_ATTR	0x0003
-#define IB_MGMT_STATUS_INVALID_VALUE		0x0004
-
-/** A LID routed SMP header
+ * Subnet management MADs
  *
- * Defined in section 14.2.1.1 of the IBA.
+ *****************************************************************************
  */
-struct ib_smp_lr_hdr {
-	uint64_t mkey;
-	uint8_t reserved[32];
-} __attribute__ (( packed ));
 
-/** A directed route SMP header
+/** A subnet management header
  *
- * Defined in section 14.2.1.2 of the IBA.
+ * Defined in sections 14.2.1.1 and 14.2.1.2 of the IBA.
  */
-struct ib_smp_dr_hdr {
+struct ib_smp_hdr {
 	uint64_t mkey;
 	uint16_t slid;
 	uint16_t dlid;
 	uint8_t reserved[28];
 } __attribute__ (( packed ));
 
-/** A subnet management header */
-union ib_smp_hdr {
-	uint64_t mkey;
-	struct ib_smp_lr_hdr lr;
-	struct ib_smp_dr_hdr dr;
-} __attribute__ (( packed ));
-
 /** Subnet management class version */
 #define IB_SMP_CLASS_VERSION			1
 
@@ -242,17 +183,24 @@ union ib_smp_data {
 
 /** A subnet management directed route path */
 struct ib_smp_dr_path {
-	uint8_t reserved;
-	uint8_t hops[63];
+	uint8_t hops[64];
 } __attribute__ (( packed ));
 
-/** A subnet management MAD */
-struct ib_mad_smp {
-	struct ib_mad_hdr mad_hdr;
-	union ib_smp_hdr smp_hdr;
-	union ib_smp_data smp_data;
-	struct ib_smp_dr_path initial_path;
-	struct ib_smp_dr_path return_path;
+/** Subnet management MAD class-specific data */
+struct ib_smp_class_specific {
+	uint8_t hop_pointer;
+	uint8_t hop_count;
+} __attribute__ (( packed ));
+
+/*****************************************************************************
+ *
+ * Subnet administration MADs
+ *
+ *****************************************************************************
+ */
+
+struct ib_rmpp_hdr {
+	uint32_t raw[3];
 } __attribute__ (( packed ));
 
 struct ib_sa_hdr {
@@ -262,14 +210,10 @@ struct ib_sa_hdr {
 	uint32_t comp_mask[2];
 } __attribute__ (( packed ));
 
-struct ib_rmpp_hdr {
-	uint32_t raw[3];
-} __attribute__ (( packed ));
+#define IB_SA_ATTR_MC_MEMBER_REC		0x38
+#define IB_SA_ATTR_PATH_REC			0x35
 
-struct ib_mad_path_record {
-	struct ib_mad_hdr mad_hdr;
-	struct ib_rmpp_hdr rmpp_hdr;
-	struct ib_sa_hdr sa_hdr;
+struct ib_path_record {
 	uint32_t reserved0[2];
 	struct ib_gid dgid;
 	struct ib_gid sgid;
@@ -285,10 +229,10 @@ struct ib_mad_path_record {
 	uint32_t reserved2[35];
 } __attribute__ (( packed ));
 
-struct ib_mad_mc_member_record {
-	struct ib_mad_hdr mad_hdr;
-	struct ib_rmpp_hdr rmpp_hdr;
-	struct ib_sa_hdr sa_hdr;
+#define IB_SA_PATH_REC_DGID			(1<<2)
+#define IB_SA_PATH_REC_SGID			(1<<3)
+
+struct ib_mc_member_record {
 	struct ib_gid mgid;
 	struct ib_gid port_gid;
 	uint32_t qkey;
@@ -305,9 +249,6 @@ struct ib_mad_mc_member_record {
 	uint32_t reserved1[37];
 } __attribute__ (( packed ));
 
-#define IB_SA_ATTR_MC_MEMBER_REC		0x38
-#define IB_SA_ATTR_PATH_REC			0x35
-
 #define IB_SA_MCMEMBER_REC_MGID			(1<<0)
 #define IB_SA_MCMEMBER_REC_PORT_GID		(1<<1)
 #define IB_SA_MCMEMBER_REC_QKEY			(1<<2)
@@ -327,14 +268,96 @@ struct ib_mad_mc_member_record {
 #define IB_SA_MCMEMBER_REC_JOIN_STATE		(1<<16)
 #define IB_SA_MCMEMBER_REC_PROXY_JOIN		(1<<17)
 
-#define IB_SA_PATH_REC_DGID			(1<<2)
-#define IB_SA_PATH_REC_SGID			(1<<3)
+union ib_sa_data {
+	struct ib_path_record path_record;
+	struct ib_mc_member_record mc_member_record;
+} __attribute__ (( packed ));
+
+/*****************************************************************************
+ *
+ * MADs
+ *
+ *****************************************************************************
+ */
+
+/** Management datagram class_specific data */
+union ib_mad_class_specific {
+	uint16_t raw;
+	struct ib_smp_class_specific smp;
+} __attribute__ (( packed ));
+
+/** A management datagram common header
+ *
+ * Defined in section 13.4.2 of the IBA.
+ */
+struct ib_mad_hdr {
+	uint8_t base_version;
+	uint8_t mgmt_class;
+	uint8_t class_version;
+	uint8_t method;
+	uint16_t status;
+	union ib_mad_class_specific class_specific;
+	uint32_t tid[2];
+	uint16_t attr_id;
+	uint8_t reserved[2];
+	uint32_t attr_mod;
+} __attribute__ (( packed ));
+
+/* Management base version */
+#define IB_MGMT_BASE_VERSION			1
+
+/* Management classes */
+#define IB_MGMT_CLASS_SUBN_LID_ROUTED		0x01
+#define IB_MGMT_CLASS_SUBN_DIRECTED_ROUTE	0x81
+#define IB_MGMT_CLASS_SUBN_ADM			0x03
+#define IB_MGMT_CLASS_PERF_MGMT			0x04
+#define IB_MGMT_CLASS_BM			0x05
+#define IB_MGMT_CLASS_DEVICE_MGMT		0x06
+#define IB_MGMT_CLASS_CM			0x07
+#define IB_MGMT_CLASS_SNMP			0x08
+#define IB_MGMT_CLASS_VENDOR_RANGE2_START	0x30
+#define IB_MGMT_CLASS_VENDOR_RANGE2_END		0x4F
+
+/* Management methods */
+#define IB_MGMT_METHOD_GET			0x01
+#define IB_MGMT_METHOD_SET			0x02
+#define IB_MGMT_METHOD_GET_RESP			0x81
+#define IB_MGMT_METHOD_SEND			0x03
+#define IB_MGMT_METHOD_TRAP			0x05
+#define IB_MGMT_METHOD_REPORT			0x06
+#define IB_MGMT_METHOD_REPORT_RESP		0x86
+#define IB_MGMT_METHOD_TRAP_REPRESS		0x07
+#define IB_MGMT_METHOD_DELETE			0x15
+
+/* Status codes */
+#define IB_MGMT_STATUS_OK			0x0000
+#define IB_MGMT_STATUS_BAD_VERSION		0x0001
+#define IB_MGMT_STATUS_UNSUPPORTED_METHOD	0x0002
+#define IB_MGMT_STATUS_UNSUPPORTED_METHOD_ATTR	0x0003
+#define IB_MGMT_STATUS_INVALID_VALUE		0x0004
+
+/** A subnet management MAD */
+struct ib_mad_smp {
+	struct ib_mad_hdr mad_hdr;
+	struct ib_smp_hdr smp_hdr;
+	union ib_smp_data smp_data;
+	struct ib_smp_dr_path initial_path;
+	struct ib_smp_dr_path return_path;
+} __attribute__ (( packed ));
+
+/** A subnet administration MAD */
+struct ib_mad_sa {
+	struct ib_mad_hdr mad_hdr;
+	struct ib_rmpp_hdr rmpp_hdr;
+	struct ib_sa_hdr sa_hdr;
+	union ib_sa_data sa_data;
+} __attribute__ (( packed ));
 
+/** A management datagram */
 union ib_mad {
 	struct ib_mad_hdr hdr;
 	struct ib_mad_smp smp;
-	struct ib_mad_path_record path_record;
-	struct ib_mad_mc_member_record mc_member_record;
+	struct ib_mad_sa sa;
 	uint8_t bytes[256];
 } __attribute__ (( packed ));