Skip to contentSkip to conversation

opensm: Reduce heap consumption by multicast routing tables (MFTs)

PATCH Applied Hal Rosenstock 4 participants 12 messages Oct 23, 2009 – Oct 30, 2009 4 messages not in the archive InfiniBand

lore
Share & tools
lore

Hal Rosenstock

author
#0 lore raw

PATCH v5 1/1 opensm: Reduce heap consumption by multicast routing tables (MFTs)

Heap memory consumption by the unicast and multicast routing tables can be reduced.

This patch is analagous to the previous patch doing this for the unicast routing tables (LFTs).

Using valgrind --tool=massif (for heap profiling), there are couple of place ->38.75% (11,206,656B) 0x43267E: osm_switch_new (osm_switch.c:134) ->12.89% (3,728,256B) 0x40F8C9: osm_mcast_tbl_init (osm_mcast_tbl.c:96)

osm_mcast_tbl_init (osm_mcast_tbl.c:96):
p_tbl->p_mask_tbl = malloc(p_tbl->num_entries *
(IB_MCAST_POSITION_MAX +
1) * IB_MCAST_MASK_SIZE / 8);

num_entries above is set based on the switch's MulticastFDBCap (indicated in it's SM class SwitchInfo attribute).

MFTs are only be increased in size and are never reduced in size. If a realloc for MFT fails, it is treated as a fatal error and OpenSM is exited.

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

---
Changes since v4:
Incorporated osm_mcast_tbl change to make max_mlid_ho be maximum MLID configured rather than max table size

Changes since v3:
Renamed mft_size to mft_depth and added description in osm_mcast_tbl.h Removed vestigial realloc mask tbl call in osm_dump.c Simplified max_mlid determination in alloc_mfts
Added return value to osm_mcast_tbl_realloc_mask_tbl
Added return value to alloc_mfts
Handle alloc_mfts failure in osm_mcast_mgr_process/process_mgroups Renamed osm_mcast_tbl_realloc_mask_tbl to osm_mcast_tbl_realloc In osm_mcast_tbl_realloc, simplified mft_depth calculation

Changes since v2:
MFT allocation during routing preparation rather than on table access

Changes since v1:
MFT allocation based on actual MLID requests

opensm/include/opensm/osm_mcast_tbl.h +39 -6
index 276b7f7..5c36f2a 100644
@@ -1,6 +1,6 @@
 /*
  * Copyright (c) 2004, 2005 Voltaire, Inc. All rights reserved.
- * Copyright (c) 2002-2005 Mellanox Technologies LTD. All rights reserved.
+ * Copyright (c) 2002-2009 Mellanox Technologies LTD. All rights reserved.
  * Copyright (c) 1996-2003 Intel Corporation. All rights reserved.
  * Copyright (c) 2009 HNR Consulting. All rights reserved.
  *

@@ -75,6 +75,7 @@ typedef struct osm_mcast_fwdbl {
 	int16_t max_block_in_use;
 	uint16_t num_entries;
 	uint16_t max_mlid_ho;
+	uint16_t mft_depth;
 	uint16_t(*p_mask_tbl)[][IB_MCAST_POSITION_MAX];
 } osm_mcast_tbl_t;
 /*

@@ -96,10 +97,14 @@ typedef struct osm_mcast_fwdbl {
 *		Number of entries in the table (aka number of MLIDs supported).
 *
 *	max_mlid_ho
-*		Maximum MLID (host order) configured in the multicast port mask
+*		Maximum MLID (host order) for the currently allocated multicast
+*		port mask table.
+*
+*	mft_depth
+*		Number of MLIDs in the currently allocated multicast port mask
 *		table.
 *
-*	pp_mask_tbl
+*	p_mask_tbl
 *		Pointer to a two dimensional array of port_masks for this switch.
 *		The first dimension is MLID, the second dimension is mask position.
 *		This pointer is null for switches that do not support multicast.

@@ -116,8 +121,8 @@ typedef struct osm_mcast_fwdbl {
 *
 * SYNOPSIS
 */
-ib_api_status_t osm_mcast_tbl_init(IN osm_mcast_tbl_t * p_tbl,
-				   IN uint8_t num_ports, IN uint16_t capacity);
+void osm_mcast_tbl_init(IN osm_mcast_tbl_t * p_tbl, IN uint8_t num_ports,
+			IN uint16_t capacity);
 /*
 * PARAMETERS
 *	num_ports

@@ -128,7 +133,7 @@ ib_api_status_t osm_mcast_tbl_init(IN osm_mcast_tbl_t * p_tbl,
 *		by this switch.
 *
 * RETURN VALUE
-*	IB_SUCCESS on success.
+*	None.
 *
 * NOTES
 *

@@ -160,6 +165,34 @@ void osm_mcast_tbl_delete(IN osm_mcast_tbl_t ** pp_tbl);
 * SEE ALSO
 *********/
 
+/****f* OpenSM: Forwarding Table/osm_mcast_tbl_realloc
+* NAME
+*	osm_mcast_tbl_realloc
+*
+* DESCRIPTION
+*	This function reallocates the multicast port mask table if necessary.
+*
+* SYNOPSIS
+*/
+int
+osm_mcast_tbl_realloc(IN osm_mcast_tbl_t * p_tbl, IN uintn_t mlid_offset);
+/*
+* PARAMETERS
+*
+*	p_tbl
+*		[in] Pointer to the Multicast Forwarding Table object.
+*
+*	mlid_offset
+*		[in] Offset of MLID being accessed.
+*
+* RETURN VALUE
+*	Returns 0 on success and non-zero value otherwise.
+*
+* NOTES
+*
+* SEE ALSO
+*/
+
 /****f* OpenSM: Forwarding Table/osm_mcast_tbl_destroy
 * NAME
 *	osm_mcast_tbl_destroy
opensm/opensm/osm_mcast_mgr.c +42 -0

60 lines not on this page · raw message

opensm/opensm/osm_mcast_tbl.c +51 -36

162 lines not on this page · raw message

opensm/opensm/osm_switch.c +2 -5
index d2123a4..c912b03 100644
@@ -136,9 +136,8 @@ osm_switch_t *osm_switch_new(IN osm_node_t * p_node,
 
 	memset(p_sw->p_prof, 0, sizeof(*p_sw->p_prof) * num_ports);
 
-	if (osm_mcast_tbl_init(&p_sw->mcast_tbl, osm_node_get_num_physp(p_node),
-			       cl_ntoh16(p_si->mcast_cap)))
-		goto err;
+	osm_mcast_tbl_init(&p_sw->mcast_tbl, osm_node_get_num_physp(p_node),
+			   cl_ntoh16(p_si->mcast_cap));
 
 	for (port_num = 0; port_num < num_ports; port_num++)
 		osm_port_prof_construct(&p_sw->p_prof[port_num]);

@@ -507,7 +506,6 @@ static int alloc_lft(IN osm_switch_t * p_sw, uint16_t lids)
 		p_sw->lft = new_lft;
 		p_sw->lft_size = lft_size;
 	}
-
 	return 0;
 }
 

@@ -548,7 +546,6 @@ int osm_switch_prepare_path_rebuild(IN osm_switch_t * p_sw, IN uint16_t max_lids
 		p_sw->num_hops = max_lids + 1;
 	}
 	p_sw->max_lid_ho = max_lids;
-
 	return 0;
 }

--
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

A message that is not in the archive (sent off-list or not yet received): [email protected]

Applied
#2 lore raw
Quoted 16 lines · Hal Rosenstock

Heap memory consumption by the unicast and multicast routing tables can be reduced.

This patch is analagous to the previous patch doing this for the unicast routing tables (LFTs).

Using valgrind --tool=massif (for heap profiling), there are couple of place ->38.75% (11,206,656B) 0x43267E: osm_switch_new (osm_switch.c:134) ->12.89% (3,728,256B) 0x40F8C9: osm_mcast_tbl_init (osm_mcast_tbl.c:96)

osm_mcast_tbl_init (osm_mcast_tbl.c:96):
p_tbl->p_mask_tbl = malloc(p_tbl->num_entries *
(IB_MCAST_POSITION_MAX +
1) * IB_MCAST_MASK_SIZE / 8);

Quoted 7 lines

num_entries above is set based on the switch's MulticastFDBCap (indicated in it's SM class SwitchInfo attribute).

MFTs are only be increased in size and are never reduced in size. If a realloc for MFT fails, it is treated as a fatal error and OpenSM is exited.

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

Applied. Thanks.

I have some comments (see below) and will send the fixes as subsequent patch(es). Please reply over comments and/or patches if needed.

Quoted 138 lines

---
Changes since v4:
Incorporated osm_mcast_tbl change to make max_mlid_ho be maximum MLID configured rather than max table size

Changes since v3:
Renamed mft_size to mft_depth and added description in osm_mcast_tbl.h Removed vestigial realloc mask tbl call in osm_dump.c Simplified max_mlid determination in alloc_mfts
Added return value to osm_mcast_tbl_realloc_mask_tbl
Added return value to alloc_mfts
Handle alloc_mfts failure in osm_mcast_mgr_process/process_mgroups Renamed osm_mcast_tbl_realloc_mask_tbl to osm_mcast_tbl_realloc In osm_mcast_tbl_realloc, simplified mft_depth calculation

Changes since v2:
MFT allocation during routing preparation rather than on table access

Changes since v1:
MFT allocation based on actual MLID requests

opensm/include/opensm/osm_mcast_tbl.h +39 -6
index 276b7f7..5c36f2a 100644
@@ -1,6 +1,6 @@
 /*
  * Copyright (c) 2004, 2005 Voltaire, Inc. All rights reserved.
- * Copyright (c) 2002-2005 Mellanox Technologies LTD. All rights reserved.
+ * Copyright (c) 2002-2009 Mellanox Technologies LTD. All rights reserved.
  * Copyright (c) 1996-2003 Intel Corporation. All rights reserved.
  * Copyright (c) 2009 HNR Consulting. All rights reserved.
  *

@@ -75,6 +75,7 @@ typedef struct osm_mcast_fwdbl {
 	int16_t max_block_in_use;
 	uint16_t num_entries;
 	uint16_t max_mlid_ho;
+	uint16_t mft_depth;
 	uint16_t(*p_mask_tbl)[][IB_MCAST_POSITION_MAX];
 } osm_mcast_tbl_t;
 /*

@@ -96,10 +97,14 @@ typedef struct osm_mcast_fwdbl {
 *		Number of entries in the table (aka number of MLIDs supported).
 *
 *	max_mlid_ho
-*		Maximum MLID (host order) configured in the multicast port mask
+*		Maximum MLID (host order) for the currently allocated multicast
+*		port mask table.
+*
+*	mft_depth
+*		Number of MLIDs in the currently allocated multicast port mask
 *		table.
 *
-*	pp_mask_tbl
+*	p_mask_tbl
 *		Pointer to a two dimensional array of port_masks for this switch.
 *		The first dimension is MLID, the second dimension is mask position.
 *		This pointer is null for switches that do not support multicast.

@@ -116,8 +121,8 @@ typedef struct osm_mcast_fwdbl {
 *
 * SYNOPSIS
 */
-ib_api_status_t osm_mcast_tbl_init(IN osm_mcast_tbl_t * p_tbl,
-				   IN uint8_t num_ports, IN uint16_t capacity);
+void osm_mcast_tbl_init(IN osm_mcast_tbl_t * p_tbl, IN uint8_t num_ports,
+			IN uint16_t capacity);
 /*
 * PARAMETERS
 *	num_ports

@@ -128,7 +133,7 @@ ib_api_status_t osm_mcast_tbl_init(IN osm_mcast_tbl_t * p_tbl,
 *		by this switch.
 *
 * RETURN VALUE
-*	IB_SUCCESS on success.
+*	None.
 *
 * NOTES
 *

@@ -160,6 +165,34 @@ void osm_mcast_tbl_delete(IN osm_mcast_tbl_t ** pp_tbl);
 * SEE ALSO
 *********/
 
+/****f* OpenSM: Forwarding Table/osm_mcast_tbl_realloc
+* NAME
+*	osm_mcast_tbl_realloc
+*
+* DESCRIPTION
+*	This function reallocates the multicast port mask table if necessary.
+*
+* SYNOPSIS
+*/
+int
+osm_mcast_tbl_realloc(IN osm_mcast_tbl_t * p_tbl, IN uintn_t mlid_offset);
+/*
+* PARAMETERS
+*
+*	p_tbl
+*		[in] Pointer to the Multicast Forwarding Table object.
+*
+*	mlid_offset
+*		[in] Offset of MLID being accessed.
+*
+* RETURN VALUE
+*	Returns 0 on success and non-zero value otherwise.
+*
+* NOTES
+*
+* SEE ALSO
+*/
+
 /****f* OpenSM: Forwarding Table/osm_mcast_tbl_destroy
 * NAME
 *	osm_mcast_tbl_destroy
opensm/opensm/osm_mcast_mgr.c +17 -0
index 0ee689c..a5e9758 100644
@@ -1043,6 +1043,36 @@ static int mcast_mgr_set_mftables(osm_sm_t * sm)
 	return ret;
 }
 
+static int alloc_mfts(osm_sm_t * sm)
+{
+	int i;
+	cl_map_item_t *item;
+	osm_switch_t *p_sw;
+	int max_mlid = 0;
+
+	for (i = sm->p_subn->max_mcast_lid_ho - IB_LID_MCAST_START_HO; i >= 0;
+	     i--) {
+		if (sm->p_subn->mgroups[i]) {
+			max_mlid = i + IB_LID_MCAST_START_HO;
+			break;
+		}
+	}
+
+	if (max_mlid == 0)
+		return 0;

opensm/opensm/osm_mcast_mgr.c static int mcast_mgr_set_mftables(osm_sm_t * sm) · line 1072

Quoted 10 lines
opensm/opensm/osm_mcast_mgr.c +10 -0
+
+	/* Now, walk switches and (re)allocate multicast tables */
+	for (item = cl_qmap_head(&sm->p_subn->sw_guid_tbl);
+	     item != cl_qmap_end(&sm->p_subn->sw_guid_tbl);
+	     item = cl_qmap_next(item)) {
+		p_sw = (osm_switch_t *)item;
+		if (osm_mcast_tbl_realloc(&p_sw->mcast_tbl,
+					  max_mlid - IB_LID_MCAST_START_HO))
+			return -1;
+	}

A variable 'max_mlid' is not actually needed in this function - you are initializing this as 'mlid_max = i + IB_LID_MCAST_START_NO' and then using as 'mlid_max - IB_LID_MCAST_START_HO'. Instead you could just use 'i' as it is.

Quoted 91 lines
opensm/opensm/osm_mcast_mgr.c +15 -0
+	return 0;
+}
+
 /**********************************************************************
  **********************************************************************/
 int osm_mcast_mgr_process(osm_sm_t * sm)

@@ -1063,6 +1093,12 @@ int osm_mcast_mgr_process(osm_sm_t * sm)
 		goto exit;
 	}
 
+	if (alloc_mfts(sm)) {
+		OSM_LOG(sm->p_log, OSM_LOG_ERROR,
+			"ERR 0A07: alloc_mfts failed\n");
+		goto exit;
+	}
+
 	for (i = 0; i <= sm->p_subn->max_mcast_lid_ho - IB_LID_MCAST_START_HO;
 	     i++)
 		if (sm->p_subn->mgroups[i] || sm->mlids_req[i])

@@ -1101,6 +1137,12 @@ int osm_mcast_mgr_process_mgroups(osm_sm_t * sm)
 		goto exit;
 	}
 
+	if (alloc_mfts(sm)) {
+		OSM_LOG(sm->p_log, OSM_LOG_ERROR,
+			"ERR 0A09: alloc_mfts failed\n");
+		goto exit;
+	}
+
 	for (i = 0; i <= sm->mlids_req_max; i++) {
 		if (!sm->mlids_req[i])
 			continue;
opensm/opensm/osm_mcast_tbl.c +4 -23
index bdea416..be2181d 100644
@@ -1,6 +1,6 @@
 /*
  * Copyright (c) 2004-2006 Voltaire, Inc. All rights reserved.
- * Copyright (c) 2002-2005 Mellanox Technologies LTD. All rights reserved.
+ * Copyright (c) 2002-2009 Mellanox Technologies LTD. All rights reserved.
  * Copyright (c) 1996-2003 Intel Corporation. All rights reserved.
  * Copyright (c) 2009 HNR Consulting. All rights reserved.
  *

@@ -53,8 +53,8 @@
 
 /**********************************************************************
  **********************************************************************/
-ib_api_status_t osm_mcast_tbl_init(IN osm_mcast_tbl_t * p_tbl,
-				   IN uint8_t num_ports, IN uint16_t capacity)
+void osm_mcast_tbl_init(IN osm_mcast_tbl_t * p_tbl, IN uint8_t num_ports,
+			IN uint16_t capacity)
 {
 	CL_ASSERT(p_tbl);
 	CL_ASSERT(num_ports);

@@ -68,7 +68,7 @@ ib_api_status_t osm_mcast_tbl_init(IN osm_mcast_tbl_t * p_tbl,
 		   This switch apparently doesn't support multicast.
 		   Everything is initialized to zero already, so return.
 		 */
-		return IB_SUCCESS;
+		return;
 	}
 
 	p_tbl->num_entries = capacity;

@@ -80,25 +80,6 @@ ib_api_status_t osm_mcast_tbl_init(IN osm_mcast_tbl_t * p_tbl,
 	p_tbl->max_block = (uint16_t) ((ROUNDUP(p_tbl->num_entries,
 						IB_MCAST_BLOCK_SIZE) /
 					IB_MCAST_BLOCK_SIZE) - 1);
-
-	/*
-	   The number of bytes needed in the mask table is:
-	   The (maximum bit mask 'position' + 1) times the
-	   number of bytes in each bit mask times the
-	   number of MLIDs supported by the table.
-
-	   We must always allocate the array with the maximum position
-	   since it is (and must be) defined that way the table structure
-	   in order to create a pointer to a two dimensional array.
-	 */
-	p_tbl->p_mask_tbl = calloc(p_tbl->num_entries,
-				   (IB_MCAST_POSITION_MAX +
-				    1) * IB_MCAST_MASK_SIZE / 8);
-
-	if (p_tbl->p_mask_tbl == NULL)
-		return IB_INSUFFICIENT_MEMORY;
-
-	return IB_SUCCESS;
 }
 
 /**********************************************************************

opensm/opensm/osm_mcast_tbl.c void osm_mcast_tbl_set(IN osm_mcast_tbl_t * p_tbl, IN uint16_t mlid_ho, · line 105

Quoted 7 lines
opensm/opensm/osm_mcast_tbl.c +2 -2
@@ -120,8 +101,8 @@ void osm_mcast_tbl_set(IN osm_mcast_tbl_t * p_tbl, IN uint16_t mlid_ho,
 
 	CL_ASSERT(p_tbl);
 	CL_ASSERT(mlid_ho >= IB_LID_MCAST_START_HO);
-	CL_ASSERT(mlid_ho <= (uint16_t) (IB_LID_MCAST_START_HO +
-					 p_tbl->num_entries - 1));
+	CL_ASSERT(mlid_ho <= p_tbl->max_mlid_ho);
+	CL_ASSERT(mlid_ho - IB_LID_MCAST_START_HO < p_tbl->mft_depth);

Isn't it the same check since
'p_tbl->max_mlid_ho = p_tbl->mft_depth + IB_LID_MCAST_START_HO - 1'?

Quoted 33 lines
opensm/opensm/osm_mcast_tbl.c +24 -2
 	CL_ASSERT(p_tbl->p_mask_tbl);
 
 	mlid_offset = mlid_ho - IB_LID_MCAST_START_HO;

@@ -133,8 +114,41 @@ void osm_mcast_tbl_set(IN osm_mcast_tbl_t * p_tbl, IN uint16_t mlid_ho,
 
 	if (block_num > p_tbl->max_block_in_use)
 		p_tbl->max_block_in_use = (uint16_t) block_num;
-	if (mlid_ho > p_tbl->max_mlid_ho)
-		p_tbl->max_mlid_ho = mlid_ho;
+}
+
+/**********************************************************************
+ **********************************************************************/
+int
+osm_mcast_tbl_realloc(IN osm_mcast_tbl_t * p_tbl, IN uintn_t mlid_offset)
+{
+	size_t mft_depth, size;
+	uint16_t (*p_mask_tbl)[][IB_MCAST_POSITION_MAX];
+
+	if (mlid_offset < p_tbl->mft_depth)
+		return 0;
+
+	/*
+	   The number of bytes needed in the mask table is:
+	   The (maximum bit mask 'position' + 1) times the
+	   number of bytes in each bit mask times the
+	   number of MLIDs supported by the table.
+
+	   We must always allocate the array with the maximum position
+	   since it is (and must be) defined that way the table structure
+	   in order to create a pointer to a two dimensional array.
+	 */
+	mft_depth = (mlid_offset / IB_MCAST_BLOCK_SIZE + 1) * IB_MCAST_BLOCK_SIZE;

opensm/opensm/osm_mcast_tbl.c void osm_mcast_tbl_set(IN osm_mcast_tbl_t * p_tbl, IN uint16_t mlid_ho, · line 150

Quoted 10 lines
opensm/opensm/osm_mcast_tbl.c +10 -0
+	size = mft_depth * (IB_MCAST_POSITION_MAX + 1) * IB_MCAST_MASK_SIZE / 8;
+	p_mask_tbl = realloc(p_tbl->p_mask_tbl, size);
+	if (!p_mask_tbl)
+		return -1;
+	memset((uint8_t *)p_mask_tbl + p_tbl->mft_depth * (IB_MCAST_POSITION_MAX + 1) * IB_MCAST_MASK_SIZE / 8,
+	       0,
+	       size - p_tbl->mft_depth * (IB_MCAST_POSITION_MAX + 1) * IB_MCAST_MASK_SIZE / 8);
+	p_tbl->p_mask_tbl = p_mask_tbl;
+	p_tbl->mft_depth = mft_depth;
+	p_tbl->max_mlid_ho = mft_depth + IB_LID_MCAST_START_HO - 1;

Wouldn't it be more accurate/efficient to set max_mlid_ho as 'mlid_offset + IB_LID_MCAST_START_HO - 1'?

Quoted 5 lines
opensm/opensm/osm_mcast_tbl.c +1 -0
+	return 0;
 }
 
 /**********************************************************************

opensm/opensm/osm_mcast_tbl.c boolean_t osm_mcast_tbl_is_port(IN const osm_mcast_tbl_t * p_tbl, · line 173

Quoted 10 lines
opensm/opensm/osm_mcast_tbl.c +3 -2
@@ -152,10 +166,11 @@ boolean_t osm_mcast_tbl_is_port(IN const osm_mcast_tbl_t * p_tbl,
 		CL_ASSERT(port_num <=
 			  (p_tbl->max_position + 1) * IB_MCAST_MASK_SIZE);
 		CL_ASSERT(mlid_ho >= IB_LID_MCAST_START_HO);
-		CL_ASSERT(mlid_ho <= (uint16_t) (IB_LID_MCAST_START_HO +
-						 p_tbl->num_entries - 1));
+		CL_ASSERT(mlid_ho <= p_tbl->max_mlid_ho);
 
 		mlid_offset = mlid_ho - IB_LID_MCAST_START_HO;
+		if (mlid_offset >= p_tbl->mft_depth)
+			return FALSE;

This duplicates CL_ASSERT() above. Looking on how this function is used I don't see why we this check should be introduced. Do you?

Quoted 4 lines
opensm/opensm/osm_mcast_tbl.c +0 -0
 		mask_offset = port_num / IB_MCAST_MASK_SIZE;
 		bit_mask = cl_ntoh16((uint16_t)
 				     (1 << (port_num % IB_MCAST_MASK_SIZE)));

opensm/opensm/osm_mcast_tbl.c boolean_t osm_mcast_tbl_is_any_port(IN const osm_mcast_tbl_t * p_tbl, · line 202

Quoted 10 lines
opensm/opensm/osm_mcast_tbl.c +3 -2
@@ -180,10 +195,11 @@ boolean_t osm_mcast_tbl_is_any_port(IN const osm_mcast_tbl_t * p_tbl,
 
 	if (p_tbl->p_mask_tbl) {
 		CL_ASSERT(mlid_ho >= IB_LID_MCAST_START_HO);
-		CL_ASSERT(mlid_ho <= (uint16_t) (IB_LID_MCAST_START_HO +
-						 p_tbl->num_entries - 1));
+		CL_ASSERT(mlid_ho <= p_tbl->max_mlid_ho);
 
 		mlid_offset = mlid_ho - IB_LID_MCAST_START_HO;
+		if (mlid_offset >= p_tbl->mft_depth)
+			return FALSE;

Ditto.

opensm/opensm/osm_mcast_tbl.c ib_api_status_t osm_mcast_tbl_set_block(IN osm_mcast_tbl_t * p_tbl, · line 233

Quoted 11 lines
opensm/opensm/osm_mcast_tbl.c +1 -2
 		for (position = 0; position <= p_tbl->max_position; position++)
 			result |= (*p_tbl->p_mask_tbl)[mlid_offset][position];

@@ -213,8 +229,7 @@ ib_api_status_t osm_mcast_tbl_set_block(IN osm_mcast_tbl_t * p_tbl,
 
 	mlid_start_ho = (uint16_t) (block_num * IB_MCAST_BLOCK_SIZE);
 
-	if (mlid_start_ho + IB_MCAST_BLOCK_SIZE - 1 >
-	    p_tbl->num_entries + IB_LID_MCAST_START_HO - 1)
+	if (mlid_start_ho + IB_MCAST_BLOCK_SIZE - 1 > p_tbl->mft_depth)
 		return IB_INVALID_PARAMETER;

I see that 'mlid_start_ho' is actually a mlid offset. Was a previous check wrong?

Quoted 13 lines
opensm/opensm/osm_mcast_tbl.c +0 -3
 	for (i = 0; i < IB_MCAST_BLOCK_SIZE; i++)

@@ -223,9 +238,6 @@ ib_api_status_t osm_mcast_tbl_set_block(IN osm_mcast_tbl_t * p_tbl,
 	if (block_num > p_tbl->max_block_in_use)
 		p_tbl->max_block_in_use = (uint16_t) block_num;
 
-	if (mlid_start_ho + IB_MCAST_BLOCK_SIZE - 1 > p_tbl->max_mlid_ho)
-		p_tbl->max_mlid_ho = mlid_start_ho + IB_MCAST_BLOCK_SIZE - 1;
-
 	return IB_SUCCESS;
 }

opensm/opensm/osm_mcast_tbl.c void osm_mcast_tbl_clear_mlid(IN osm_mcast_tbl_t * p_tbl, IN uint16_t mlid_ho) · line 257

Quoted 5 lines
opensm/opensm/osm_mcast_tbl.c +2 -0
@@ -241,6 +253,8 @@ void osm_mcast_tbl_clear_mlid(IN osm_mcast_tbl_t * p_tbl, IN uint16_t mlid_ho)
 
 	if (p_tbl->p_mask_tbl && (mlid_ho <= p_tbl->max_mlid_ho)) {
 		mlid_offset = mlid_ho - IB_LID_MCAST_START_HO;
+		if (mlid_offset >= p_tbl->mft_depth)
+			return;

This seems redundant for me after 'mlid_ho <= p_tbl->max_mlid_ho' check above.

Sasha

Quoted 44 lines
opensm/opensm/osm_mcast_tbl.c +1 -0
 		for (i = 0; i <= p_tbl->max_position; i++)
 			(*p_tbl->p_mask_tbl)[mlid_offset][i] = 0;
 	}

@@ -257,6 +271,7 @@ boolean_t osm_mcast_tbl_get_block(IN osm_mcast_tbl_t * p_tbl,
 
 	CL_ASSERT(p_tbl);
 	CL_ASSERT(p_block);
+	CL_ASSERT(block_num * IB_MCAST_BLOCK_SIZE <= p_tbl->mft_depth);
 
 	if (block_num > p_tbl->max_block_in_use)
 		return FALSE;
opensm/opensm/osm_switch.c +2 -6
index d2123a4..c912b03 100644
@@ -136,9 +136,8 @@ osm_switch_t *osm_switch_new(IN osm_node_t * p_node,
 
 	memset(p_sw->p_prof, 0, sizeof(*p_sw->p_prof) * num_ports);
 
-	if (osm_mcast_tbl_init(&p_sw->mcast_tbl, osm_node_get_num_physp(p_node),
-			       cl_ntoh16(p_si->mcast_cap)))
-		goto err;
+	osm_mcast_tbl_init(&p_sw->mcast_tbl, osm_node_get_num_physp(p_node),
+			   cl_ntoh16(p_si->mcast_cap));
 
 	for (port_num = 0; port_num < num_ports; port_num++)
 		osm_port_prof_construct(&p_sw->p_prof[port_num]);

@@ -507,7 +506,6 @@ static int alloc_lft(IN osm_switch_t * p_sw, uint16_t lids)
 		p_sw->lft = new_lft;
 		p_sw->lft_size = lft_size;
 	}
-
 	return 0;
 }
 

@@ -548,7 +546,6 @@ int osm_switch_prepare_path_rebuild(IN osm_switch_t * p_sw, IN uint16_t max_lids
 		p_sw->num_hops = max_lids + 1;
 	}
 	p_sw->max_lid_ho = max_lids;
-
 	return 0;
 }
 
--
Quoted 4 lines

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

--
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

#3 lore raw

PATCH 1/1 opensm/osm_mcast_mgr: simplify alloc_mfts()

Remove not actually needed 'max_mlid' variable from alloc_mft() function.

Signed-off-by Sasha Khapyorsky <sashak-smomgflXvOZWk0Htik3J/[email protected]>
---
 opensm/opensm/osm_mcast_mgr.c |   14 ++++----------
 1 files changed, 4 insertions(+), 10 deletions(-)
opensm/opensm/osm_mcast_mgr.c +4 -10
index a5e9758..faa0c4b 100644
@@ -1048,17 +1048,12 @@ static int alloc_mfts(osm_sm_t * sm)
 	int i;
 	cl_map_item_t *item;
 	osm_switch_t *p_sw;
-	int max_mlid = 0;
 
 	for (i = sm->p_subn->max_mcast_lid_ho - IB_LID_MCAST_START_HO; i >= 0;
-	     i--) {
-		if (sm->p_subn->mgroups[i]) {
-			max_mlid = i + IB_LID_MCAST_START_HO;
+	     i--)
+		if (sm->p_subn->mgroups[i])
 			break;
-		}
-	}
-
-	if (max_mlid == 0)
+	if (i < 0)
 		return 0;
 
 	/* Now, walk switches and (re)allocate multicast tables */

@@ -1066,8 +1061,7 @@ static int alloc_mfts(osm_sm_t * sm)
 	     item != cl_qmap_end(&sm->p_subn->sw_guid_tbl);
 	     item = cl_qmap_next(item)) {
 		p_sw = (osm_switch_t *)item;
-		if (osm_mcast_tbl_realloc(&p_sw->mcast_tbl,
-					  max_mlid - IB_LID_MCAST_START_HO))
+		if (osm_mcast_tbl_realloc(&p_sw->mcast_tbl, i))
 			return -1;
 	}
 	return 0;
Signature · 6 lines
1.6.5.1

--
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
#4 lore raw

PATCH 1/1 opensm/osm_mcast_tbl: remove redundant checks

In reply to Sasha Khapyorsky

Remove some redundant checks introduced by MFTs reallocation patch.

Signed-off-by Sasha Khapyorsky <sashak-smomgflXvOZWk0Htik3J/[email protected]>
---
 opensm/opensm/osm_mcast_tbl.c |   12 ++----------
 1 files changed, 2 insertions(+), 10 deletions(-)
opensm/opensm/osm_mcast_tbl.c +2 -10
index 6cc97b1..b5ae6f2 100644
@@ -99,11 +99,9 @@ void osm_mcast_tbl_set(IN osm_mcast_tbl_t * p_tbl, IN uint16_t mlid_ho,
 	uintn_t bit_mask;
 	int16_t block_num;
 
-	CL_ASSERT(p_tbl);
+	CL_ASSERT(p_tbl && p_tbl->p_mask_tbl);
 	CL_ASSERT(mlid_ho >= IB_LID_MCAST_START_HO);
 	CL_ASSERT(mlid_ho <= p_tbl->max_mlid_ho);
-	CL_ASSERT(mlid_ho - IB_LID_MCAST_START_HO < p_tbl->mft_depth);
-	CL_ASSERT(p_tbl->p_mask_tbl);
 
 	mlid_offset = mlid_ho - IB_LID_MCAST_START_HO;
 	mask_offset = port / IB_MCAST_MASK_SIZE;

@@ -168,8 +166,6 @@ boolean_t osm_mcast_tbl_is_port(IN const osm_mcast_tbl_t * p_tbl,
 		CL_ASSERT(mlid_ho <= p_tbl->max_mlid_ho);
 
 		mlid_offset = mlid_ho - IB_LID_MCAST_START_HO;
-		if (mlid_offset >= p_tbl->mft_depth)
-			return FALSE;
 		mask_offset = port_num / IB_MCAST_MASK_SIZE;
 		bit_mask = cl_ntoh16((uint16_t)
 				     (1 << (port_num % IB_MCAST_MASK_SIZE)));

@@ -197,8 +193,6 @@ boolean_t osm_mcast_tbl_is_any_port(IN const osm_mcast_tbl_t * p_tbl,
 		CL_ASSERT(mlid_ho <= p_tbl->max_mlid_ho);
 
 		mlid_offset = mlid_ho - IB_LID_MCAST_START_HO;
-		if (mlid_offset >= p_tbl->mft_depth)
-			return FALSE;
 
 		for (position = 0; position <= p_tbl->max_position; position++)
 			result |= (*p_tbl->p_mask_tbl)[mlid_offset][position];

@@ -250,10 +244,8 @@ void osm_mcast_tbl_clear_mlid(IN osm_mcast_tbl_t * p_tbl, IN uint16_t mlid_ho)
 	CL_ASSERT(p_tbl);
 	CL_ASSERT(mlid_ho >= IB_LID_MCAST_START_HO);
 
-	if (p_tbl->p_mask_tbl && (mlid_ho <= p_tbl->max_mlid_ho)) {
+	if (p_tbl->p_mask_tbl && mlid_ho <= p_tbl->max_mlid_ho) {
 		mlid_offset = mlid_ho - IB_LID_MCAST_START_HO;
-		if (mlid_offset >= p_tbl->mft_depth)
-			return;
 		for (i = 0; i <= p_tbl->max_position; i++)
 			(*p_tbl->p_mask_tbl)[mlid_offset][i] = 0;
 	}
Signature · 6 lines
1.6.5.1

--
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
#5 lore raw

Re: [PATCH] opensm/osm_mcast_tbl: remove redundant checks

Quoted 2 lines · Sasha Khapyorsky

Remove some redundant checks introduced by MFTs reallocation patch.

See below.

Quoted 23 lines
Signed-off-by Sasha Khapyorsky <sashak-smomgflXvOZWk0Htik3J/[email protected]>

---
 opensm/opensm/osm_mcast_tbl.c |   12 ++----------

 1 files changed, 2 insertions(+), 10 deletions(-)
opensm/opensm/osm_mcast_tbl.c +1 -3
index 6cc97b1..b5ae6f2 100644
@@ -99,11 +99,9 @@ void osm_mcast_tbl_set(IN osm_mcast_tbl_t * p_tbl, IN uint16_t mlid_ho,
        uintn_t bit_mask;
        int16_t block_num;
 
-       CL_ASSERT(p_tbl);
+       CL_ASSERT(p_tbl && p_tbl->p_mask_tbl);
        CL_ASSERT(mlid_ho >= IB_LID_MCAST_START_HO);
        CL_ASSERT(mlid_ho <= p_tbl->max_mlid_ho);
-       CL_ASSERT(mlid_ho - IB_LID_MCAST_START_HO < p_tbl->mft_depth);
-       CL_ASSERT(p_tbl->p_mask_tbl);
 
        mlid_offset = mlid_ho - IB_LID_MCAST_START_HO;
        mask_offset = port / IB_MCAST_MASK_SIZE;

opensm/opensm/osm_mcast_tbl.c boolean_t osm_mcast_tbl_is_port(IN const osm_mcast_tbl_t * p_tbl, · line 172

Quoted 5 lines
opensm/opensm/osm_mcast_tbl.c +0 -2
@@ -168,8 +166,6 @@ boolean_t osm_mcast_tbl_is_port(IN const osm_mcast_tbl_t * p_tbl,
                CL_ASSERT(mlid_ho <= p_tbl->max_mlid_ho);
 
                mlid_offset = mlid_ho - IB_LID_MCAST_START_HO;
-               if (mlid_offset >= p_tbl->mft_depth)
-                       return FALSE;

This check is needed for group removal to work properly.

Quoted 13 lines
opensm/opensm/osm_mcast_tbl.c +0 -2
                mask_offset = port_num / IB_MCAST_MASK_SIZE;
                bit_mask = cl_ntoh16((uint16_t)
                                     (1 << (port_num % IB_MCAST_MASK_SIZE)));

@@ -197,8 +193,6 @@ boolean_t osm_mcast_tbl_is_any_port(IN const osm_mcast_tbl_t * p_tbl,
                CL_ASSERT(mlid_ho <= p_tbl->max_mlid_ho);
 
                mlid_offset = mlid_ho - IB_LID_MCAST_START_HO;
-               if (mlid_offset >= p_tbl->mft_depth)
-                       return FALSE;
 
                for (position = 0; position <= p_tbl->max_position; position++)
                        result |= (*p_tbl->p_mask_tbl)[mlid_offset][position];

opensm/opensm/osm_mcast_tbl.c void osm_mcast_tbl_clear_mlid(IN osm_mcast_tbl_t * p_tbl, IN uint16_t mlid_ho) · line 256

Quoted 8 lines
opensm/opensm/osm_mcast_tbl.c +1 -3
@@ -250,10 +244,8 @@ void osm_mcast_tbl_clear_mlid(IN osm_mcast_tbl_t * p_tbl, IN uint16_t mlid_ho)
        CL_ASSERT(p_tbl);
        CL_ASSERT(mlid_ho >= IB_LID_MCAST_START_HO);
 
-       if (p_tbl->p_mask_tbl && (mlid_ho <= p_tbl->max_mlid_ho)) {
+       if (p_tbl->p_mask_tbl && mlid_ho <= p_tbl->max_mlid_ho) {
                mlid_offset = mlid_ho - IB_LID_MCAST_START_HO;
-               if (mlid_offset >= p_tbl->mft_depth)
-                       return;

This should be done the same as osm_mcast_tbl_is_port.

-- Hal

Quoted 11 lines
opensm/opensm/osm_mcast_tbl.c +0 -1
                for (i = 0; i <= p_tbl->max_position; i++)
                        (*p_tbl->p_mask_tbl)[mlid_offset][i] = 0;
        }
--

1.6.5.1

--
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

--
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

#6 lore raw

PATCH 1/1 opensm/mcast_tbl: set max_mlid_ho as actually configured mlid

In reply to Sasha Khapyorsky

Set switch's mcast table max_mlid_ho as actually configured mlid for the switch instead of a value extended to MFT block size aligned. This makes mlid handling checks to be more reliable.

Signed-off-by Sasha Khapyorsky <sashak-smomgflXvOZWk0Htik3J/[email protected]>
---
 opensm/opensm/osm_mcast_tbl.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)
opensm/opensm/osm_mcast_tbl.c +3 -2
index b5ae6f2..eee9290 100644
@@ -122,7 +122,7 @@ int osm_mcast_tbl_realloc(IN osm_mcast_tbl_t * p_tbl, IN uintn_t mlid_offset)
 	uint16_t (*p_mask_tbl)[][IB_MCAST_POSITION_MAX];
 
 	if (mlid_offset < p_tbl->mft_depth)
-		return 0;
+		goto done;
 
 	/*
 	   The number of bytes needed in the mask table is:

@@ -144,7 +144,8 @@ int osm_mcast_tbl_realloc(IN osm_mcast_tbl_t * p_tbl, IN uintn_t mlid_offset)
 	       size - p_tbl->mft_depth * (IB_MCAST_POSITION_MAX + 1) * IB_MCAST_MASK_SIZE / 8);
 	p_tbl->p_mask_tbl = p_mask_tbl;
 	p_tbl->mft_depth = mft_depth;
-	p_tbl->max_mlid_ho = mft_depth + IB_LID_MCAST_START_HO - 1;
+done:
+	p_tbl->max_mlid_ho = mlid_offset + IB_LID_MCAST_START_HO - 1;
 	return 0;
 }
Signature · 6 lines
1.6.5.1

--
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
#7 lore raw

Re: [PATCH] opensm/mcast_tbl: set max_mlid_ho as actually configured mlid

Sasha, Hal,

I don't follow this thread completely but it seems this last patch causes this assertion for me...

OpenSM $ mlid_ho 49152; p_tbl->max_mlid_ho 49152
mlid_ho 49154; p_tbl->max_mlid_ho 49153
opensm: osm_mcast_tbl.c:107: osm_mcast_tbl_set: Assertion `mlid_ho <= p_tbl->max_mlid_ho' failed. Aborted (core dumped)

[I added the prints of mlid_ho and p_tbl->max_mlid_ho.]

so...

	p_tbl->max_mlid_ho = mft_depth + IB_LID_MCAST_START_HO - 1;

Seems to be right,
Ira

Quoted 41 lines · On Thu, 29 Oct 2009 21:19:45 +0200 Sasha Khapyorsky…

Set switch's mcast table max_mlid_ho as actually configured mlid for the switch instead of a value extended to MFT block size aligned. This makes mlid handling checks to be more reliable.

Signed-off-by Sasha Khapyorsky <sashak-smomgflXvOZWk0Htik3J/[email protected]>
---
 opensm/opensm/osm_mcast_tbl.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)
opensm/opensm/osm_mcast_tbl.c +3 -2
index b5ae6f2..eee9290 100644
@@ -122,7 +122,7 @@ int osm_mcast_tbl_realloc(IN osm_mcast_tbl_t * p_tbl, IN uintn_t mlid_offset)
 	uint16_t (*p_mask_tbl)[][IB_MCAST_POSITION_MAX];
 
 	if (mlid_offset < p_tbl->mft_depth)
-		return 0;
+		goto done;
 
 	/*
 	   The number of bytes needed in the mask table is:

@@ -144,7 +144,8 @@ int osm_mcast_tbl_realloc(IN osm_mcast_tbl_t * p_tbl, IN uintn_t mlid_offset)
 	       size - p_tbl->mft_depth * (IB_MCAST_POSITION_MAX + 1) * IB_MCAST_MASK_SIZE / 8);
 	p_tbl->p_mask_tbl = p_mask_tbl;
 	p_tbl->mft_depth = mft_depth;
-	p_tbl->max_mlid_ho = mft_depth + IB_LID_MCAST_START_HO - 1;
+done:
+	p_tbl->max_mlid_ho = mlid_offset + IB_LID_MCAST_START_HO - 1;
 	return 0;
 }
Signature · 6 lines
1.6.5.1

--
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
Signature · 9 lines
Ira Weiny
Math Programmer/Computer Scientist
Lawrence Livermore National Lab
925-423-8008
[email protected]
--
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

A message that is not in the archive (sent off-list or not yet received): [email protected]

#9 lore raw

Re: [PATCH] opensm/mcast_tbl: set max_mlid_ho as actually configured mlid

Hi Ira,

Thanks for reporting.

Quoted 5 lines · Ira Weiny

OpenSM $ mlid_ho 49152; p_tbl->max_mlid_ho 49152
mlid_ho 49154; p_tbl->max_mlid_ho 49153
opensm: osm_mcast_tbl.c:107: osm_mcast_tbl_set: Assertion `mlid_ho <= p_tbl->max_mlid_ho' failed. Aborted (core dumped)

Small fix:

opensm/opensm/osm_mcast_tbl.c +1 -1
index eee9290..0a45904 100644
@@ -145,7 +145,7 @@ int osm_mcast_tbl_realloc(IN osm_mcast_tbl_t * p_tbl, IN uintn_t mlid_offset)
 	p_tbl->p_mask_tbl = p_mask_tbl;
 	p_tbl->mft_depth = mft_depth;
 done:
-	p_tbl->max_mlid_ho = mlid_offset + IB_LID_MCAST_START_HO - 1;
+	p_tbl->max_mlid_ho = mlid_offset + IB_LID_MCAST_START_HO;
 	return 0;
 }

Does it work for you?

Sasha
--
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

#10 lore raw

Re: [PATCH] opensm/mcast_tbl: set max_mlid_ho as actually configured mlid

Quoted 27 lines · On Fri, 30 Oct 2009 01:02:46 +0200 Sasha Khapyorsky…

Hi Ira,

Thanks for reporting.

Quoted 5 lines · Ira Weiny

OpenSM $ mlid_ho 49152; p_tbl->max_mlid_ho 49152
mlid_ho 49154; p_tbl->max_mlid_ho 49153
opensm: osm_mcast_tbl.c:107: osm_mcast_tbl_set: Assertion `mlid_ho <= p_tbl->max_mlid_ho' failed. Aborted (core dumped)

Small fix:

opensm/opensm/osm_mcast_tbl.c +1 -1
index eee9290..0a45904 100644
@@ -145,7 +145,7 @@ int osm_mcast_tbl_realloc(IN osm_mcast_tbl_t * p_tbl, IN uintn_t mlid_offset)
 	p_tbl->p_mask_tbl = p_mask_tbl;
 	p_tbl->mft_depth = mft_depth;
 done:
-	p_tbl->max_mlid_ho = mlid_offset + IB_LID_MCAST_START_HO - 1;
+	p_tbl->max_mlid_ho = mlid_offset + IB_LID_MCAST_START_HO;
 	return 0;
 }
Quoted 1 line

Does it work for you?

Yes that fixes the problem.

Thanks,
Ira

Signature · 9 lines
Ira Weiny
Math Programmer/Computer Scientist
Lawrence Livermore National Lab
925-423-8008
[email protected]
--
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

A message that is not in the archive (sent off-list or not yet received): [email protected]

#13 lore raw
In reply to Sasha Khapyorsky
Quoted 346 lines · Sasha Khapyorsky
Quoted 16 lines · Hal Rosenstock

Heap memory consumption by the unicast and multicast routing tables can be reduced.

This patch is analagous to the previous patch doing this for the unicast routing tables (LFTs).

Using valgrind --tool=massif (for heap profiling), there are couple of place ->38.75% (11,206,656B) 0x43267E: osm_switch_new (osm_switch.c:134) ->12.89% (3,728,256B) 0x40F8C9: osm_mcast_tbl_init (osm_mcast_tbl.c:96)

osm_mcast_tbl_init (osm_mcast_tbl.c:96):
        p_tbl->p_mask_tbl = malloc(p_tbl->num_entries * (IB_MCAST_POSITION_MAX +
                                    1) * IB_MCAST_MASK_SIZE / 8);

Quoted 7 lines

num_entries above is set based on the switch's MulticastFDBCap (indicated in it's SM class SwitchInfo attribute).

MFTs are only be increased in size and are never reduced in size. If a realloc for MFT fails, it is treated as a fatal error and OpenSM is exited.

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

Applied. Thanks.

I have some comments (see below) and will send the fixes as subsequent patch(es). Please reply over comments and/or patches if needed.

Quoted 138 lines

---
Changes since v4:
Incorporated osm_mcast_tbl change to make max_mlid_ho be maximum MLID configured rather than max table size

Changes since v3:
Renamed mft_size to mft_depth and added description in osm_mcast_tbl.h Removed vestigial realloc mask tbl call in osm_dump.c Simplified max_mlid determination in alloc_mfts
Added return value to osm_mcast_tbl_realloc_mask_tbl
Added return value to alloc_mfts
Handle alloc_mfts failure in osm_mcast_mgr_process/process_mgroups Renamed osm_mcast_tbl_realloc_mask_tbl to osm_mcast_tbl_realloc In osm_mcast_tbl_realloc, simplified mft_depth calculation

Changes since v2:
MFT allocation during routing preparation rather than on table access

Changes since v1:
MFT allocation based on actual MLID requests

opensm/include/opensm/osm_mcast_tbl.h +39 -6
index 276b7f7..5c36f2a 100644
@@ -1,6 +1,6 @@
  /*
   * Copyright (c) 2004, 2005 Voltaire, Inc. All rights reserved.
- * Copyright (c) 2002-2005 Mellanox Technologies LTD. All rights reserved.
+ * Copyright (c) 2002-2009 Mellanox Technologies LTD. All rights reserved.
   * Copyright (c) 1996-2003 Intel Corporation. All rights reserved.
   * Copyright (c) 2009 HNR Consulting. All rights reserved.
   *

@@ -75,6 +75,7 @@ typedef struct osm_mcast_fwdbl {
       int16_t max_block_in_use;
       uint16_t num_entries;
       uint16_t max_mlid_ho;
+     uint16_t mft_depth;
       uint16_t(*p_mask_tbl)[][IB_MCAST_POSITION_MAX];
  } osm_mcast_tbl_t;
  /*

@@ -96,10 +97,14 @@ typedef struct osm_mcast_fwdbl {
  *            Number of entries in the table (aka number of MLIDs supported).
  *
  *    max_mlid_ho
-*            Maximum MLID (host order) configured in the multicast port mask
+*            Maximum MLID (host order) for the currently allocated multicast
+*            port mask table.
+*
+*    mft_depth
+*            Number of MLIDs in the currently allocated multicast port mask
  *            table.
  *
-*    pp_mask_tbl
+*    p_mask_tbl
  *            Pointer to a two dimensional array of port_masks for this switch.
  *            The first dimension is MLID, the second dimension is mask position.
  *            This pointer is null for switches that do not support multicast.

@@ -116,8 +121,8 @@ typedef struct osm_mcast_fwdbl {
  *
  * SYNOPSIS
  */
-ib_api_status_t osm_mcast_tbl_init(IN osm_mcast_tbl_t * p_tbl,
-                                IN uint8_t num_ports, IN uint16_t capacity);
+void osm_mcast_tbl_init(IN osm_mcast_tbl_t * p_tbl, IN uint8_t num_ports,
+                     IN uint16_t capacity);
  /*
  * PARAMETERS
  *    num_ports

@@ -128,7 +133,7 @@ ib_api_status_t osm_mcast_tbl_init(IN osm_mcast_tbl_t * p_tbl,
  *            by this switch.
  *
  * RETURN VALUE
-*    IB_SUCCESS on success.
+*    None.
  *
  * NOTES
  *

@@ -160,6 +165,34 @@ void osm_mcast_tbl_delete(IN osm_mcast_tbl_t ** pp_tbl);
  * SEE ALSO
  *********/
 
+/****f* OpenSM: Forwarding Table/osm_mcast_tbl_realloc
+* NAME
+*    osm_mcast_tbl_realloc
+*
+* DESCRIPTION
+*    This function reallocates the multicast port mask table if necessary.
+*
+* SYNOPSIS
+*/
+int
+osm_mcast_tbl_realloc(IN osm_mcast_tbl_t * p_tbl, IN uintn_t mlid_offset);
+/*
+* PARAMETERS
+*
+*    p_tbl
+*            [in] Pointer to the Multicast Forwarding Table object.
+*
+*    mlid_offset
+*            [in] Offset of MLID being accessed.
+*
+* RETURN VALUE
+*    Returns 0 on success and non-zero value otherwise.
+*
+* NOTES
+*
+* SEE ALSO
+*/
+
  /****f* OpenSM: Forwarding Table/osm_mcast_tbl_destroy
  * NAME
  *    osm_mcast_tbl_destroy
opensm/opensm/osm_mcast_mgr.c +17 -0
index 0ee689c..a5e9758 100644
@@ -1043,6 +1043,36 @@ static int mcast_mgr_set_mftables(osm_sm_t * sm)
       return ret;
  }
 
+static int alloc_mfts(osm_sm_t * sm)
+{
+     int i;
+     cl_map_item_t *item;
+     osm_switch_t *p_sw;
+     int max_mlid = 0;
+
+     for (i = sm->p_subn->max_mcast_lid_ho - IB_LID_MCAST_START_HO; i >= 0;
+          i--) {
+             if (sm->p_subn->mgroups[i]) {
+                     max_mlid = i + IB_LID_MCAST_START_HO;
+                     break;
+             }
+     }
+
+     if (max_mlid == 0)
+             return 0;

opensm/opensm/osm_mcast_mgr.c static int mcast_mgr_set_mftables(osm_sm_t * sm) · line 1072

Quoted 10 lines
opensm/opensm/osm_mcast_mgr.c +10 -0
+
+     /* Now, walk switches and (re)allocate multicast tables */
+     for (item = cl_qmap_head(&sm->p_subn->sw_guid_tbl);
+          item != cl_qmap_end(&sm->p_subn->sw_guid_tbl);
+          item = cl_qmap_next(item)) {
+             p_sw = (osm_switch_t *)item;
+             if (osm_mcast_tbl_realloc(&p_sw->mcast_tbl,
+                                       max_mlid - IB_LID_MCAST_START_HO))
+                     return -1;
+     }

A variable 'max_mlid' is not actually needed in this function - you are initializing this as 'mlid_max = i + IB_LID_MCAST_START_NO' and then using as 'mlid_max - IB_LID_MCAST_START_HO'. Instead you could just use 'i' as it is.

Quoted 91 lines
opensm/opensm/osm_mcast_mgr.c +15 -0
+     return 0;
+}
+
  /**********************************************************************
   **********************************************************************/
  int osm_mcast_mgr_process(osm_sm_t * sm)

@@ -1063,6 +1093,12 @@ int osm_mcast_mgr_process(osm_sm_t * sm)
               goto exit;
       }
 
+     if (alloc_mfts(sm)) {
+             OSM_LOG(sm->p_log, OSM_LOG_ERROR,
+                     "ERR 0A07: alloc_mfts failed\n");
+             goto exit;
+     }
+
       for (i = 0; i <= sm->p_subn->max_mcast_lid_ho - IB_LID_MCAST_START_HO;
            i++)
               if (sm->p_subn->mgroups[i] || sm->mlids_req[i])

@@ -1101,6 +1137,12 @@ int osm_mcast_mgr_process_mgroups(osm_sm_t * sm)
               goto exit;
       }
 
+     if (alloc_mfts(sm)) {
+             OSM_LOG(sm->p_log, OSM_LOG_ERROR,
+                     "ERR 0A09: alloc_mfts failed\n");
+             goto exit;
+     }
+
       for (i = 0; i <= sm->mlids_req_max; i++) {
               if (!sm->mlids_req[i])
                       continue;
opensm/opensm/osm_mcast_tbl.c +3 -3
index bdea416..be2181d 100644
@@ -1,6 +1,6 @@
  /*
   * Copyright (c) 2004-2006 Voltaire, Inc. All rights reserved.
- * Copyright (c) 2002-2005 Mellanox Technologies LTD. All rights reserved.
+ * Copyright (c) 2002-2009 Mellanox Technologies LTD. All rights reserved.
   * Copyright (c) 1996-2003 Intel Corporation. All rights reserved.
   * Copyright (c) 2009 HNR Consulting. All rights reserved.
   *

@@ -53,8 +53,8 @@
 
  /**********************************************************************
   **********************************************************************/
-ib_api_status_t osm_mcast_tbl_init(IN osm_mcast_tbl_t * p_tbl,
-                                IN uint8_t num_ports, IN uint16_t capacity)
+void osm_mcast_tbl_init(IN osm_mcast_tbl_t * p_tbl, IN uint8_t num_ports,
+                     IN uint16_t capacity)
  {
       CL_ASSERT(p_tbl);
       CL_ASSERT(num_ports);

@@ -68,7 +68,7 @@ ib_api_status_t osm_mcast_tbl_init(IN osm_mcast_tbl_t * p_tbl,

                 This switch apparently doesn't support multicast. Everything is initialized to zero already, so return. */
-             return IB_SUCCESS;
+             return;
      }

      p_tbl->num_entries = capacity;

opensm/opensm/osm_mcast_tbl.c +0 -0
@@ -80,25 +80,6 @@ ib_api_status_t osm_mcast_tbl_init(IN osm_mcast_tbl_t * p_tbl,
       p_tbl->max_block = (uint16_t) ((ROUNDUP(p_tbl->num_entries,

                                              IB_MCAST_BLOCK_SIZE) / IB_MCAST_BLOCK_SIZE) - 1);

opensm/opensm/osm_mcast_tbl.c +0 -19
-
-     /*
-        The number of bytes needed in the mask table is:
-        The (maximum bit mask 'position' + 1) times the
-        number of bytes in each bit mask times the
-        number of MLIDs supported by the table.
-
-        We must always allocate the array with the maximum position
-        since it is (and must be) defined that way the table structure
-        in order to create a pointer to a two dimensional array.
-      */
-     p_tbl->p_mask_tbl = calloc(p_tbl->num_entries,
-                                (IB_MCAST_POSITION_MAX +
-                                 1) * IB_MCAST_MASK_SIZE / 8);
-
-     if (p_tbl->p_mask_tbl == NULL)
-             return IB_INSUFFICIENT_MEMORY;
-
-     return IB_SUCCESS;

 }

 /**********************************************************************

opensm/opensm/osm_mcast_tbl.c void osm_mcast_tbl_set(IN osm_mcast_tbl_t * p_tbl, IN uint16_t mlid_ho, · line 105

Quoted 7 lines
opensm/opensm/osm_mcast_tbl.c +2 -2
@@ -120,8 +101,8 @@ void osm_mcast_tbl_set(IN osm_mcast_tbl_t * p_tbl, IN uint16_t mlid_ho,
 
       CL_ASSERT(p_tbl);
       CL_ASSERT(mlid_ho >= IB_LID_MCAST_START_HO);
-     CL_ASSERT(mlid_ho <= (uint16_t) (IB_LID_MCAST_START_HO +
-                                      p_tbl->num_entries - 1));
+     CL_ASSERT(mlid_ho <= p_tbl->max_mlid_ho);
+     CL_ASSERT(mlid_ho - IB_LID_MCAST_START_HO < p_tbl->mft_depth);

Isn't it the same check since
'p_tbl->max_mlid_ho = p_tbl->mft_depth + IB_LID_MCAST_START_HO - 1'?

Quoted 33 lines
opensm/opensm/osm_mcast_tbl.c +24 -2
       CL_ASSERT(p_tbl->p_mask_tbl);
 
       mlid_offset = mlid_ho - IB_LID_MCAST_START_HO;

@@ -133,8 +114,41 @@ void osm_mcast_tbl_set(IN osm_mcast_tbl_t * p_tbl, IN uint16_t mlid_ho,
 
       if (block_num > p_tbl->max_block_in_use)
               p_tbl->max_block_in_use = (uint16_t) block_num;
-     if (mlid_ho > p_tbl->max_mlid_ho)
-             p_tbl->max_mlid_ho = mlid_ho;
+}
+
+/**********************************************************************
+ **********************************************************************/
+int
+osm_mcast_tbl_realloc(IN osm_mcast_tbl_t * p_tbl, IN uintn_t mlid_offset)
+{
+     size_t mft_depth, size;
+     uint16_t (*p_mask_tbl)[][IB_MCAST_POSITION_MAX];
+
+     if (mlid_offset < p_tbl->mft_depth)
+             return 0;
+
+     /*
+        The number of bytes needed in the mask table is:
+        The (maximum bit mask 'position' + 1) times the
+        number of bytes in each bit mask times the
+        number of MLIDs supported by the table.
+
+        We must always allocate the array with the maximum position
+        since it is (and must be) defined that way the table structure
+        in order to create a pointer to a two dimensional array.
+      */
+     mft_depth = (mlid_offset / IB_MCAST_BLOCK_SIZE + 1) * IB_MCAST_BLOCK_SIZE;

opensm/opensm/osm_mcast_tbl.c void osm_mcast_tbl_set(IN osm_mcast_tbl_t * p_tbl, IN uint16_t mlid_ho, · line 150

Quoted 10 lines
opensm/opensm/osm_mcast_tbl.c +10 -0
+     size = mft_depth * (IB_MCAST_POSITION_MAX + 1) * IB_MCAST_MASK_SIZE / 8;
+     p_mask_tbl = realloc(p_tbl->p_mask_tbl, size);
+     if (!p_mask_tbl)
+             return -1;
+     memset((uint8_t *)p_mask_tbl + p_tbl->mft_depth * (IB_MCAST_POSITION_MAX + 1) * IB_MCAST_MASK_SIZE / 8,
+            0,
+            size - p_tbl->mft_depth * (IB_MCAST_POSITION_MAX + 1) * IB_MCAST_MASK_SIZE / 8);
+     p_tbl->p_mask_tbl = p_mask_tbl;
+     p_tbl->mft_depth = mft_depth;
+     p_tbl->max_mlid_ho = mft_depth + IB_LID_MCAST_START_HO - 1;

Wouldn't it be more accurate/efficient to set max_mlid_ho as 'mlid_offset + IB_LID_MCAST_START_HO - 1'?

Quoted 12 lines
opensm/opensm/osm_mcast_tbl.c +1 -0
+     return 0;
  }
 
  /**********************************************************************

@@ -152,10 +166,11 @@ boolean_t osm_mcast_tbl_is_port(IN const osm_mcast_tbl_t * p_tbl,

              CL_ASSERT(port_num <=
                        (p_tbl->max_position + 1) * IB_MCAST_MASK_SIZE); CL_ASSERT(mlid_ho >= IB_LID_MCAST_START_HO);
-             CL_ASSERT(mlid_ho <= (uint16_t) (IB_LID_MCAST_START_HO +
-                                              p_tbl->num_entries - 1));
+             CL_ASSERT(mlid_ho <= p_tbl->max_mlid_ho);

Quoted 5 lines
Quoted 3 lines

              mlid_offset = mlid_ho - IB_LID_MCAST_START_HO;
+             if (mlid_offset >= p_tbl->mft_depth)
+                     return FALSE;

This duplicates CL_ASSERT() above.

Not quite. There's a difference between max_mlid_ho and mft_depth.

Quoted 2 lines

Looking on how this function is used
I don't see why we this check should be introduced. Do you?

It's needed for group removal to work properly.

Quoted 11 lines
Quoted 11 lines

              mask_offset = port_num / IB_MCAST_MASK_SIZE; bit_mask = cl_ntoh16((uint16_t)
                                   (1 << (port_num % IB_MCAST_MASK_SIZE)));

opensm/opensm/osm_mcast_tbl.c +1 -2
@@ -180,10 +195,11 @@ boolean_t osm_mcast_tbl_is_any_port(IN const osm_mcast_tbl_t * p_tbl,
 
       if (p_tbl->p_mask_tbl) {
               CL_ASSERT(mlid_ho >= IB_LID_MCAST_START_HO);
-             CL_ASSERT(mlid_ho <= (uint16_t) (IB_LID_MCAST_START_HO +
-                                              p_tbl->num_entries - 1));
+             CL_ASSERT(mlid_ho <= p_tbl->max_mlid_ho);
Quoted 5 lines

opensm/opensm/osm_mcast_tbl.c boolean_t osm_mcast_tbl_is_port(IN const osm_mcast_tbl_t * p_tbl, · line 173

Quoted 3 lines
opensm/opensm/osm_mcast_tbl.c +2 -0
               mlid_offset = mlid_ho - IB_LID_MCAST_START_HO;
+             if (mlid_offset >= p_tbl->mft_depth)
+                     return FALSE;

Ditto.

See above.

Quoted 7 lines
Quoted 7 lines
opensm/opensm/osm_mcast_tbl.c +0 -0
               for (position = 0; position <= p_tbl->max_position; position++)
                       result |= (*p_tbl->p_mask_tbl)[mlid_offset][position];

@@ -213,8 +229,7 @@ ib_api_status_t osm_mcast_tbl_set_block(IN osm_mcast_tbl_t * p_tbl,
 
       mlid_start_ho = (uint16_t) (block_num * IB_MCAST_BLOCK_SIZE);
Quoted 7 lines

opensm/opensm/osm_mcast_tbl.c ib_api_status_t osm_mcast_tbl_set_block(IN osm_mcast_tbl_t * p_tbl, · line 233

Quoted 4 lines
opensm/opensm/osm_mcast_tbl.c +1 -2
-     if (mlid_start_ho + IB_MCAST_BLOCK_SIZE - 1 >
-         p_tbl->num_entries + IB_LID_MCAST_START_HO - 1)
+     if (mlid_start_ho + IB_MCAST_BLOCK_SIZE - 1 > p_tbl->mft_depth)
               return IB_INVALID_PARAMETER;

I see that 'mlid_start_ho' is actually a mlid offset. Was a previous check wrong?

This check ensures the whole MFT block (in terms of SM attribute use) is allocated.
It isn't needed as the table allocation is done in these units. Prior to this, I think there was a potential boundary condition so it was needed.

Quoted 14 lines
Quoted 14 lines
opensm/opensm/osm_mcast_tbl.c +0 -3
       for (i = 0; i < IB_MCAST_BLOCK_SIZE; i++)

@@ -223,9 +238,6 @@ ib_api_status_t osm_mcast_tbl_set_block(IN osm_mcast_tbl_t * p_tbl,
       if (block_num > p_tbl->max_block_in_use)
               p_tbl->max_block_in_use = (uint16_t) block_num;
 
-     if (mlid_start_ho + IB_MCAST_BLOCK_SIZE - 1 > p_tbl->max_mlid_ho)
-             p_tbl->max_mlid_ho = mlid_start_ho + IB_MCAST_BLOCK_SIZE - 1;
-
       return IB_SUCCESS;
  }
 

@@ -241,6 +253,8 @@ void osm_mcast_tbl_clear_mlid(IN osm_mcast_tbl_t * p_tbl, IN uint16_t mlid_ho)
Quoted 7 lines

opensm/opensm/osm_mcast_tbl.c void osm_mcast_tbl_clear_mlid(IN osm_mcast_tbl_t * p_tbl, IN uint16_t mlid_ho) · line 257

Quoted 4 lines
opensm/opensm/osm_mcast_tbl.c +2 -0
       if (p_tbl->p_mask_tbl && (mlid_ho <= p_tbl->max_mlid_ho)) {
               mlid_offset = mlid_ho - IB_LID_MCAST_START_HO;
+             if (mlid_offset >= p_tbl->mft_depth)
+                     return;

This seems redundant for me after 'mlid_ho <= p_tbl->max_mlid_ho' check above.

See above.

-- Hal

Quoted 45 lines

Sasha

Quoted 43 lines
opensm/opensm/osm_mcast_tbl.c +1 -0
               for (i = 0; i <= p_tbl->max_position; i++)
                       (*p_tbl->p_mask_tbl)[mlid_offset][i] = 0;
       }

@@ -257,6 +271,7 @@ boolean_t osm_mcast_tbl_get_block(IN osm_mcast_tbl_t * p_tbl,
 
       CL_ASSERT(p_tbl);
       CL_ASSERT(p_block);
+     CL_ASSERT(block_num * IB_MCAST_BLOCK_SIZE <= p_tbl->mft_depth);
 
       if (block_num > p_tbl->max_block_in_use)
               return FALSE;
opensm/opensm/osm_switch.c +2 -5
index d2123a4..c912b03 100644
@@ -136,9 +136,8 @@ osm_switch_t *osm_switch_new(IN osm_node_t * p_node,
 
       memset(p_sw->p_prof, 0, sizeof(*p_sw->p_prof) * num_ports);
 
-     if (osm_mcast_tbl_init(&p_sw->mcast_tbl, osm_node_get_num_physp(p_node),
-                            cl_ntoh16(p_si->mcast_cap)))
-             goto err;
+     osm_mcast_tbl_init(&p_sw->mcast_tbl, osm_node_get_num_physp(p_node),
+                        cl_ntoh16(p_si->mcast_cap));
 
       for (port_num = 0; port_num < num_ports; port_num++)
               osm_port_prof_construct(&p_sw->p_prof[port_num]);

@@ -507,7 +506,6 @@ static int alloc_lft(IN osm_switch_t * p_sw, uint16_t lids)
               p_sw->lft = new_lft;
               p_sw->lft_size = lft_size;
       }
-
       return 0;
  }
 

@@ -548,7 +546,6 @@ int osm_switch_prepare_path_rebuild(IN osm_switch_t * p_sw, IN uint16_t max_lids
               p_sw->num_hops = max_lids + 1;
       }
       p_sw->max_lid_ho = max_lids;
-
       return 0;
  }
Quoted 10 lines
Quoted 5 lines

--
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

--
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

--
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

A message that is not in the archive (sent off-list or not yet received): f0e08f230910300647q274e4e76v70c60a978f84309c-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org

#15 lore raw

Hi Hal,

Quoted 8 lines · Hal Rosenstock
Quoted 8 lines
Quoted 8 lines
(quoted diff) +0 -0
@@ -152,10 +166,11 @@ boolean_t osm_mcast_tbl_is_port(IN const osm_mcast_tbl_t * p_tbl,

              CL_ASSERT(port_num <=
                        (p_tbl->max_position + 1) * IB_MCAST_MASK_SIZE); CL_ASSERT(mlid_ho >= IB_LID_MCAST_START_HO);
-             CL_ASSERT(mlid_ho <= (uint16_t) (IB_LID_MCAST_START_HO +
-                                              p_tbl->num_entries - 1));
+             CL_ASSERT(mlid_ho <= p_tbl->max_mlid_ho);

Quoted 7 lines
Quoted 5 lines
Quoted 3 lines

              mlid_offset = mlid_ho - IB_LID_MCAST_START_HO;
+             if (mlid_offset >= p_tbl->mft_depth)
+                     return FALSE;

This duplicates CL_ASSERT() above.

Not quite. There's a difference between max_mlid_ho and mft_depth.

And 'max_mlid_ho <= mft_depth + IB_LID_MCAST_START_HO - 1', isn't it?

Quoted 4 lines
Quoted 2 lines

Looking on how this function is used
I don't see why we this check should be introduced. Do you?

It's needed for group removal to work properly.

Could you elaborate? osm_mcast_tbl_is_port() is called only from osm_switch_recommend_mcast_path(), how is it related to group removal?

Quoted 13 lines
Quoted 13 lines
Quoted 13 lines

              mask_offset = port_num / IB_MCAST_MASK_SIZE; bit_mask = cl_ntoh16((uint16_t)
                                   (1 << (port_num % IB_MCAST_MASK_SIZE)));

opensm/opensm/osm_mcast_tbl.c +1 -2
@@ -180,10 +195,11 @@ boolean_t osm_mcast_tbl_is_any_port(IN const osm_mcast_tbl_t * p_tbl,
 
       if (p_tbl->p_mask_tbl) {
               CL_ASSERT(mlid_ho >= IB_LID_MCAST_START_HO);
-             CL_ASSERT(mlid_ho <= (uint16_t) (IB_LID_MCAST_START_HO +
-                                              p_tbl->num_entries - 1));
+             CL_ASSERT(mlid_ho <= p_tbl->max_mlid_ho);
Quoted 7 lines
Quoted 5 lines

opensm/opensm/osm_mcast_tbl.c boolean_t osm_mcast_tbl_is_port(IN const osm_mcast_tbl_t * p_tbl, · line 173

Quoted 3 lines
opensm/opensm/osm_mcast_tbl.c +2 -0
               mlid_offset = mlid_ho - IB_LID_MCAST_START_HO;
+             if (mlid_offset >= p_tbl->mft_depth)
+                     return FALSE;

Ditto.

See above.

And this function osm_mcast_tbl_is_any_port() is not called at all. Is it?

Quoted 20 lines
Quoted 12 lines
Quoted 12 lines
opensm/opensm/osm_mcast_tbl.c +0 -3
@@ -223,9 +238,6 @@ ib_api_status_t osm_mcast_tbl_set_block(IN osm_mcast_tbl_t * p_tbl,
       if (block_num > p_tbl->max_block_in_use)
               p_tbl->max_block_in_use = (uint16_t) block_num;
 
-     if (mlid_start_ho + IB_MCAST_BLOCK_SIZE - 1 > p_tbl->max_mlid_ho)
-             p_tbl->max_mlid_ho = mlid_start_ho + IB_MCAST_BLOCK_SIZE - 1;
-
       return IB_SUCCESS;
  }
 

@@ -241,6 +253,8 @@ void osm_mcast_tbl_clear_mlid(IN osm_mcast_tbl_t * p_tbl, IN uint16_t mlid_ho)
Quoted 7 lines

opensm/opensm/osm_mcast_tbl.c void osm_mcast_tbl_clear_mlid(IN osm_mcast_tbl_t * p_tbl, IN uint16_t mlid_ho) · line 257

Quoted 4 lines
opensm/opensm/osm_mcast_tbl.c +2 -0
       if (p_tbl->p_mask_tbl && (mlid_ho <= p_tbl->max_mlid_ho)) {
               mlid_offset = mlid_ho - IB_LID_MCAST_START_HO;
+             if (mlid_offset >= p_tbl->mft_depth)
+                     return;

This seems redundant for me after 'mlid_ho <= p_tbl->max_mlid_ho' check above.

Quoted 1 line

See above.

I see nothing related above. Again:

This is what we will have after your patch:

	if (p_tbl->p_mask_tbl && (mlid_ho <= p_tbl->max_mlid_ho)) {
		mlid_offset = mlid_ho - IB_LID_MCAST_START_HO;
		if (mlid_offset >= p_tbl->mft_depth)
			return;

, and assuming that:

	p_tbl->max_mlid_ho = p_tbl->mft_depth + IB_LID_MCAST_START_HO - 1

, how the second if () condition could be true?

Sasha
--
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