[ofa-general] Re: [PATCH v2] opensm: free lft_buf if it matches switch's lft

[Sasha Khapyorsky]

Hi Yevgeny,

On 14:46 Thu 20 Nov     , Yevgeny Kliteynik wrote:
>
> I can do something like the following patch, but I have
> some strange feeling that I'm missing something...

I cannot see any errors here. But probably you can use simpler approach
- just cleanup all switch's lft_buf separately after ucast_mgr is
finished (including wait_for_pending_transactions()). Something like
below (if it is fine for you I can just apply this patch).

BTW, what about to rename lft_buf to new_lft (to improve readability)?

Sasha


diff --git a/opensm/opensm/osm_state_mgr.c b/opensm/opensm/osm_state_mgr.c
index 56212fe..c810106 100644
--- a/opensm/opensm/osm_state_mgr.c
+++ b/opensm/opensm/osm_state_mgr.c
@@ -1001,6 +1001,23 @@ static void __osm_state_mgr_check_tbl_consistency(IN osm_sm_t * sm)
 	OSM_LOG_EXIT(sm->p_log);
 }
 
+static void cleanup_switch(cl_map_item_t *item, void *log)
+{
+	osm_switch_t *sw = (osm_switch_t *)item;
+
+	if (!sw->lft_buf)
+		return;
+	
+	if (memcmp(sw->lft, sw->lft_buf, IB_LID_UCAST_END_HO + 1))
+		osm_log(log, OSM_LOG_ERROR, "ERR 331D: "
+			"LFT of switch 0x%016" PRIx64 " is not up to date.\n",
+			cl_ntoh64(sw->p_node->node_info.node_guid));
+	else {
+		free(sw->lft_buf);
+		sw->lft_buf = NULL;
+	}
+}
+
 /**********************************************************************
  **********************************************************************/
 int wait_for_pending_transactions(osm_stats_t * stats)
@@ -1254,6 +1271,9 @@ _repeat_discovery:
 	if (wait_for_pending_transactions(&sm->p_subn->p_osm->stats))
 		return;
 
+	/* cleanup switch lft buffers */
+	cl_qmap_apply_func(&sm->p_subn->sw_guid_tbl, cleanup_switch, sm->p_log);
+
 	/* We are done setting all LFTs so clear the ignore existing.
 	 * From now on, as long as we are still master, we want to
 	 * take into account these lfts. */
diff --git a/opensm/opensm/osm_switch.c b/opensm/opensm/osm_switch.c
index 642dcd1..c446f4f 100644
--- a/opensm/opensm/osm_switch.c
+++ b/opensm/opensm/osm_switch.c
@@ -114,13 +114,6 @@ osm_switch_init(IN osm_switch_t * const p_sw,
 	/* Initialize the table to OSM_NO_PATH, which is "invalid port" */
 	memset(p_sw->lft, OSM_NO_PATH, IB_LID_UCAST_END_HO + 1);
 
-	p_sw->lft_buf = malloc(IB_LID_UCAST_END_HO + 1);
-	if (!p_sw->lft_buf) {
-		status = IB_INSUFFICIENT_MEMORY;
-		goto Exit;
-	}
-	memset(p_sw->lft_buf, OSM_NO_PATH, IB_LID_UCAST_END_HO + 1);
-
 	p_sw->p_prof = malloc(sizeof(*p_sw->p_prof) * num_ports);
 	if (p_sw->p_prof == NULL) {
 		status = IB_INSUFFICIENT_MEMORY;
diff --git a/opensm/opensm/osm_ucast_mgr.c b/opensm/opensm/osm_ucast_mgr.c
index 1409e15..3d47640 100644
--- a/opensm/opensm/osm_ucast_mgr.c
+++ b/opensm/opensm/osm_ucast_mgr.c
@@ -397,13 +397,6 @@ int osm_ucast_mgr_set_fwd_table(IN osm_ucast_mgr_t * const p_mgr,
 		goto Exit;
 	}
 
-	if (!p_sw->need_update &&
-	    !memcmp(p_sw->lft, p_sw->lft_buf, IB_LID_UCAST_END_HO + 1)) {
-		free(p_sw->lft_buf);
-		p_sw->lft_buf = NULL;
-		goto Exit;
-	}
-
 	for (block_id_ho = 0;
 	     osm_switch_get_lft_block(p_sw, block_id_ho, block);
 	     block_id_ho++) {