opensm/osm_pkey_mgr.c: Handle osm_pkey_tbl_new_block_get returning NULL

PATCH Superseded Hal Rosenstock 2 participants 5 messages Apr 11, 2011 – May 24, 2011 3 messages not in the archive InfiniBand

lore

Share & tools

lore

Hal Rosenstock

author

2011-04-11 19:39 UTC #0 lore raw

PATCH 2/2 opensm/osm_pkey_mgr.c: Handle osm_pkey_tbl_new_block_get returning NULL

Fixes seg fault when ib_pkey_is_invalid called in last_used_pkey_index

Introduced by commit aebd9f9d9cdd1c60c2b3784c0c03cfe8f4014019 for better last block handling

Signed-off-by Hal Rosenstock <hal-VPRAkNaXOzVWk0Htik3J/[email protected]>

---

opensm/osm_pkey_mgr.c +24 ~~-17~~

index 06d9b1e..a2021e9 100644
@@ -375,14 +375,19 @@ static int pkey_mgr_update_port(osm_log_t * p_log, osm_sm_t * sm,
 	return ret;
 }
 
-static uint16_t last_used_pkey_index(const osm_port_t * const p_port,
-				     const osm_pkey_tbl_t * p_pkey_tbl)
+static int last_used_pkey_index(const osm_port_t * const p_port,
+				const osm_pkey_tbl_t * p_pkey_tbl,
+				uint16_t * p_last_index)
 {
 	ib_pkey_table_t *last_block;
 	uint16_t index, last_index = 0;
 
+	CL_ASSERT(p_last_index);
+
 	last_block = osm_pkey_tbl_new_block_get(p_pkey_tbl,
 						p_pkey_tbl->used_blocks - 1);
+	if (!last_block)
+		return 1;
 
 	if (p_pkey_tbl->used_blocks == p_pkey_tbl->max_blocks)
 		last_index = cl_ntoh16(p_port->p_node->node_info.partition_cap) % IB_NUM_PKEY_ELEMENTS_IN_BLOCK;

@@ -395,7 +400,8 @@ static uint16_t last_used_pkey_index(const osm_port_t * const p_port,
 			break;
 	} while (index != 0);
 
-	return index;
+	*p_last_index = index;
+	return 0;
 }
 
 static int update_peer_block(osm_log_t * p_log, osm_sm_t * sm,

@@ -510,20 +516,21 @@ static int pkey_mgr_update_peer_port(osm_log_t * p_log, osm_sm_t * sm,
 		p_peer_pkey_tbl->used_blocks = peer_block_idx + 1;
 		if (p_peer_pkey_tbl->used_blocks == peer_max_blocks) {
 			/* Is last used pkey index beyond switch peer port capacity ? */
-			last_index = peer_block_idx * IB_NUM_PKEY_ELEMENTS_IN_BLOCK +
-				     last_used_pkey_index(p_port,
-							  p_peer_pkey_tbl);
-			if (cl_ntoh16(p_node->sw->switch_info.enforce_cap) <= last_index) {
-				OSM_LOG(p_log, OSM_LOG_ERROR, "ERR 0507: "
-					"Not enough pkey entries (%u <= %u) on switch 0x%016"
-					PRIx64 " port %u (%s). Clearing Enforcement bit\n",
-					cl_ntoh16(p_node->sw->switch_info.enforce_cap),
-					last_index,
-					cl_ntoh64(osm_node_get_node_guid(p_node)),
-					osm_physp_get_port_num(peer),
-					p_node->print_desc);
-				enforce = FALSE;
-				ret = -1;
+			if (!last_used_pkey_index(p_port, p_peer_pkey_tbl,
+						  &last_index)) {
+				last_index += peer_block_idx * IB_NUM_PKEY_ELEMENTS_IN_BLOCK;
+				if (cl_ntoh16(p_node->sw->switch_info.enforce_cap) <= last_index) {
+					OSM_LOG(p_log, OSM_LOG_ERROR, "ERR 0507: "
+						"Not enough pkey entries (%u <= %u) on switch 0x%016"
+						PRIx64 " port %u (%s). Clearing Enforcement bit\n",
+						cl_ntoh16(p_node->sw->switch_info.enforce_cap),
+						last_index,
+						cl_ntoh64(osm_node_get_node_guid(p_node)),
+						osm_physp_get_port_num(peer),
+						p_node->print_desc);
+					enforce = FALSE;
+					ret = -1;
+				}
 			}
 		}
 	} else {

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Alex Netes

2011-04-27 14:57 UTC #2 lore raw

In reply to a message not in the archive

Hi Hal,

Quoted 35 lines · Hal Rosenstock

Fixes seg fault when ib_pkey_is_invalid called in last_used_pkey_index

Introduced by commit aebd9f9d9cdd1c60c2b3784c0c03cfe8f4014019 for better last block handling

Signed-off-by Hal Rosenstock <hal-VPRAkNaXOzVWk0Htik3J/[email protected]>

---

opensm/osm_pkey_mgr.c +7 -2

index 06d9b1e..a2021e9 100644
@@ -375,14 +375,19 @@ static int pkey_mgr_update_port(osm_log_t * p_log, osm_sm_t * sm,
 	return ret;
 }
 
-static uint16_t last_used_pkey_index(const osm_port_t * const p_port,
-				     const osm_pkey_tbl_t * p_pkey_tbl)
+static int last_used_pkey_index(const osm_port_t * const p_port,
+				const osm_pkey_tbl_t * p_pkey_tbl,
+				uint16_t * p_last_index)
 {
 	ib_pkey_table_t *last_block;
 	uint16_t index, last_index = 0;
 
+	CL_ASSERT(p_last_index);
+
 	last_block = osm_pkey_tbl_new_block_get(p_pkey_tbl,
 						p_pkey_tbl->used_blocks - 1);
+	if (!last_block)
+		return 1;
 
 	if (p_pkey_tbl->used_blocks == p_pkey_tbl->max_blocks)
 		last_index = cl_ntoh16(p_port->p_node->node_info.partition_cap) % IB_NUM_PKEY_ELEMENTS_IN_BLOCK;

opensm/osm_pkey_mgr.c static uint16_t last_used_pkey_index(const osm_port_t * const p_port, · line 403

Quoted 5 lines

opensm/osm_pkey_mgr.c +1 -1

@@ -395,7 +400,8 @@ static uint16_t last_used_pkey_index(const osm_port_t * const p_port,
 			break;
 	} while (index != 0);
 
-	return index;
+	*p_last_index = index;

I think it should be: *p_last_index = index + 1;
You want to return the number of pkeys in the block and not the index in the array.

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Hal Rosenstock

author

2011-04-27 15:15 UTC #4 lore raw

In reply to a message not in the archive

Hi Alex,

Quoted 41 lines · Alex Netes

Hi Hal,

Quoted 38 lines · Hal Rosenstock

Fixes seg fault when ib_pkey_is_invalid called in last_used_pkey_index

Introduced by commit aebd9f9d9cdd1c60c2b3784c0c03cfe8f4014019 for better last block handling

Signed-off-by Hal Rosenstock <hal-VPRAkNaXOzVWk0Htik3J/[email protected]>

---

opensm/osm_pkey_mgr.c +7 -2

index 06d9b1e..a2021e9 100644
@@ -375,14 +375,19 @@ static int pkey_mgr_update_port(osm_log_t * p_log, osm_sm_t * sm,
 	return ret;
 }
 
-static uint16_t last_used_pkey_index(const osm_port_t * const p_port,
-				     const osm_pkey_tbl_t * p_pkey_tbl)
+static int last_used_pkey_index(const osm_port_t * const p_port,
+				const osm_pkey_tbl_t * p_pkey_tbl,
+				uint16_t * p_last_index)
 {
 	ib_pkey_table_t *last_block;
 	uint16_t index, last_index = 0;
 
+	CL_ASSERT(p_last_index);
+
 	last_block = osm_pkey_tbl_new_block_get(p_pkey_tbl,
 						p_pkey_tbl->used_blocks - 1);
+	if (!last_block)
+		return 1;
 
 	if (p_pkey_tbl->used_blocks == p_pkey_tbl->max_blocks)
 		last_index = cl_ntoh16(p_port->p_node->node_info.partition_cap) % IB_NUM_PKEY_ELEMENTS_IN_BLOCK;

@@ -395,7 +400,8 @@ static uint16_t last_used_pkey_index(const osm_port_t * const p_port,
 			break;
 	} while (index != 0);

Quoted 6 lines

opensm/osm_pkey_mgr.c static uint16_t last_used_pkey_index(const osm_port_t * const p_port, · line 403
Quoted 2 lines
opensm/osm_pkey_mgr.c +1 -1
-	return index;
+	*p_last_index = index;
I think it should be: *p_last_index = index + 1;
You want to return the number of pkeys in the block and not the index in the array.

Isn't this the last_used_pkey_index as the routine name says and not one past the last index used ?

The comparison is:

if (cl_ntoh16(p_node->sw->switch_info.enforce_cap) <= last_index)

and doesn't that take care of this ?

+ if (!last_used_pkey_index(p_port, p_peer_pkey_tbl,
+ &last_index)) {
+ last_index += peer_block_idx * IB_NUM_PKEY_ELEMENTS_IN_BLOCK;
+ if (cl_ntoh16(p_node->sw->switch_info.enforce_cap) <= last_index) {
+ OSM_LOG(p_log, OSM_LOG_ERROR, "ERR 0507: "

-- Hal
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Alex Netes

2011-04-27 15:24 UTC #6 lore raw

In reply to a message not in the archive

Quoted 60 lines · Hal Rosenstock

Hi Alex,
Quoted 41 lines · Alex Netes
Hi Hal,
Quoted 38 lines · Hal Rosenstock
Fixes seg fault when ib_pkey_is_invalid called in last_used_pkey_index
Introduced by commit aebd9f9d9cdd1c60c2b3784c0c03cfe8f4014019 for better last block handling
Signed-off-by Hal Rosenstock <hal-VPRAkNaXOzVWk0Htik3J/[email protected]>
---
opensm/osm_pkey_mgr.c +7 -2
index 06d9b1e..a2021e9 100644
@@ -375,14 +375,19 @@ static int pkey_mgr_update_port(osm_log_t * p_log, osm_sm_t * sm,
 	return ret;
 }
 
-static uint16_t last_used_pkey_index(const osm_port_t * const p_port,
-				     const osm_pkey_tbl_t * p_pkey_tbl)
+static int last_used_pkey_index(const osm_port_t * const p_port,
+				const osm_pkey_tbl_t * p_pkey_tbl,
+				uint16_t * p_last_index)
 {
 	ib_pkey_table_t *last_block;
 	uint16_t index, last_index = 0;
 
+	CL_ASSERT(p_last_index);
+
 	last_block = osm_pkey_tbl_new_block_get(p_pkey_tbl,
 						p_pkey_tbl->used_blocks - 1);
+	if (!last_block)
+		return 1;
 
 	if (p_pkey_tbl->used_blocks == p_pkey_tbl->max_blocks)
 		last_index = cl_ntoh16(p_port->p_node->node_info.partition_cap) % IB_NUM_PKEY_ELEMENTS_IN_BLOCK;
@@ -395,7 +400,8 @@ static uint16_t last_used_pkey_index(const osm_port_t * const p_port,
 			break;
 	} while (index != 0);
 
Quoted 6 lines
opensm/osm_pkey_mgr.c static uint16_t last_used_pkey_index(const osm_port_t * const p_port, · line 403
Quoted 2 lines
opensm/osm_pkey_mgr.c +1 -1
-	return index;
+	*p_last_index = index;
I think it should be: *p_last_index = index + 1;
You want to return the number of pkeys in the block and not the index in the array.
Isn't this the last_used_pkey_index as the routine name says and not one past the last index used ?
The comparison is:
if (cl_ntoh16(p_node->sw->switch_info.enforce_cap) <= last_index)
and doesn't that take care of this ?

opensm/osm_pkey_mgr.c static int pkey_mgr_update_peer_port(osm_log_t * p_log, osm_sm_t * sm, · line 523

Quoted 5 lines

opensm/osm_pkey_mgr.c +5 -0

+			if (!last_used_pkey_index(p_port, p_peer_pkey_tbl,
+						  &last_index)) {
+				last_index += peer_block_idx * IB_NUM_PKEY_ELEMENTS_IN_BLOCK;
+				if (cl_ntoh16(p_node->sw->switch_info.enforce_cap) <= last_index) {
+					OSM_LOG(p_log, OSM_LOG_ERROR, "ERR 0507: "

Yep. It does. Missed that.

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Alex Netes

2011-05-24 16:11 UTC #7 lore raw

In reply to a message not in the archive

Hi Hal,

Quoted 8 lines · Hal Rosenstock

Fixes seg fault when ib_pkey_is_invalid called in last_used_pkey_index
Introduced by commit aebd9f9d9cdd1c60c2b3784c0c03cfe8f4014019 for better last block handling
Signed-off-by Hal Rosenstock <hal-VPRAkNaXOzVWk0Htik3J/[email protected]>
---

Applied, thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html