Change LFT event to be per block/per switch rather than just per switch

-----Original Message-----
From: [email protected] [mailto:linux-rdma-

Eliminates expensive LFT comparison to determine changed blocks

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

---

include/opensm/osm_event_plugin.h +13 -0

index c9a904b..3ece2f0 100644
@@ -39,6 +39,7 @@
 #include <iba/ib_types.h>
 #include <complib/cl_qlist.h>
 #include <opensm/osm_config.h>
+#include <opensm/osm_switch.h>
 
 #ifdef __cplusplus
 #  define BEGIN_C_DECLS extern "C" {

@@ -87,6 +88,18 @@ typedef struct osm_epi_port_id {
 	char node_name[OSM_EPI_NODE_NAME_LEN];  }
 osm_epi_port_id_t;
 
+typedef enum {
+	LFT_CHANGED_LFT_TOP = (1 << 0),
+	LFT_CHANGED_BLOCK = (1 << 1)
+} osm_epi_lft_change_flags_t;
+
+typedef struct osm_epi_lft_change_event {
+	osm_switch_t *p_sw;
+	osm_epi_lft_change_flags_t flags;
+	uint16_t lft_top;
+	uint32_t block_num;
+} osm_epi_lft_change_event_t;
+
 /**

==========================================================
===============
* Port error event
* OSM_EVENT_ID_PORT_COUNTER

include/opensm/osm_madw.h +1 -0

index 5d78eaa..fd6ba7f 100644
@@ -229,6 +229,7 @@ typedef struct osm_si_context {
 	ib_net64_t node_guid;
 	boolean_t set_method;
 	boolean_t light_sweep;
+	boolean_t lft_top_change;
 } osm_si_context_t;
 /*********/
 

include/opensm/osm_switch.h +0 -1

index 6e8a87e..41ac959 100644
@@ -104,7 +104,6 @@ typedef struct osm_switch {
 	uint8_t *lft;
 	uint8_t *new_lft;
 	uint16_t lft_size;
-	unsigned lft_change;
 	osm_mcast_tbl_t mcast_tbl;
 	int32_t mft_block_num;
 	uint32_t mft_position;

opensm/osm_lin_fwd_rcv.c index renamed from opensm/osm_lin_fwd_rcv.c +0 -0

f13b9a8..dd18c09 100644

opensm/osm_lin_fwd_rcv.c +2 -0

@@ -51,6 +51,8 @@
 #define FILE_ID OSM_FILE_LIN_FWD_RCV_C
 #include <opensm/osm_switch.h>
 #include <opensm/osm_sm.h>
+#include <opensm/osm_event_plugin.h>
+#include <opensm/osm_opensm.h>
 
 void osm_lft_rcv_process(IN void *context, IN void *data)  { @@ -62,6 +64,7
 @@ void osm_lft_rcv_process(IN void *context, IN void *data)

opensm/osm_lin_fwd_rcv.c +8 -1

 	osm_lft_context_t *p_lft_context;
 	uint8_t *p_block;
 	ib_net64_t node_guid;
+	osm_epi_lft_change_event_t lft_change;
 	ib_api_status_t status;
 
 	CL_ASSERT(sm);

@@ -89,7 +92,17 @@ void osm_lft_rcv_process(IN void *context, IN void
 *data)
 			"0x%" PRIx64 "\n", cl_ntoh64(node_guid));
 	} else {
 		status = osm_switch_set_lft_block(p_sw, p_block,
 block_num);
-		if (status != IB_SUCCESS) {
+		if (status == IB_SUCCESS) {
+			if (sm->p_subn->first_time_master_sweep ==
 FALSE) {
+				lft_change.p_sw = p_sw;
+				lft_change.flags = LFT_CHANGED_BLOCK;
+				lft_change.lft_top = 0;
+				lft_change.block_num = block_num;
+				osm_opensm_report_event(sm->p_subn-

Quoted 1 line

p_osm,

+
OSM_EVENT_ID_LFT_CHANGE,

opensm/osm_lin_fwd_rcv.c +3 -0

+							&lft_change);
+			}
+		} else {
 			OSM_LOG(sm->p_log, OSM_LOG_ERROR, "ERR 0402:

"

opensm/osm_lin_fwd_rcv.c +0 -0

 				"Setting forwarding table block failed (%s)"
 				", Switch 0x%" PRIx64 " %s\n",

opensm/osm_mcast_mgr.c index renamed from opensm/osm_mcast_mgr.c +0 -0

fea0a69..135b174 100644

opensm/osm_mcast_mgr.c +1 -0

@@ -1070,6 +1070,7 @@ static void mcast_mgr_set_mfttop(IN osm_sm_t *
 sm, IN osm_switch_t * p_sw)
 		context.si_context.light_sweep = FALSE;
 		context.si_context.node_guid =
 osm_node_get_node_guid(p_node);
 		context.si_context.set_method = TRUE;
+		context.si_context.lft_top_change = FALSE;
 

opensm/osm_mcast_mgr.c +0 -0

 		status = osm_req_set(sm, p_path, (uint8_t *) & si,
 				     sizeof(si), IB_MAD_ATTR_SWITCH_INFO,

opensm/osm_node_info_rcv.c +1 -0

index cb96f29..592f2de 100644
@@ -552,6 +552,7 @@ static void ni_rcv_process_switch(IN osm_sm_t * sm,
 IN osm_node_t * p_node,
 	context.si_context.node_guid =
 osm_node_get_node_guid(p_node);
 	context.si_context.set_method = FALSE;
 	context.si_context.light_sweep = FALSE;
+	context.si_context.lft_top_change = FALSE;
 

opensm/osm_node_info_rcv.c +0 -1

 	/* Request a SwitchInfo attribute */
 	status = osm_req_get(sm, path, IB_MAD_ATTR_SWITCH_INFO, diff -
-git a/opensm/osm_state_mgr.c b/opensm/osm_state_mgr.c index

8229b06..1b73834 100644

opensm/osm_state_mgr.c +1 -0

@@ -138,6 +138,7 @@ static void state_mgr_get_sw_info(IN cl_map_item_t
 * p_object, IN void *context)
 	mad_context.si_context.node_guid =
 osm_node_get_node_guid(p_node);
 	mad_context.si_context.set_method = FALSE;
 	mad_context.si_context.light_sweep = TRUE;
+	mad_context.si_context.lft_top_change = FALSE;
 

status = osm_req_get(sm, p_dr_path,
IB_MAD_ATTR_SWITCH_INFO, 0,

opensm/osm_state_mgr.c +0 -0

 			     OSM_MSG_LIGHT_SWEEP_FAIL, &mad_context);

opensm/osm_sw_info_rcv.c index renamed from opensm/osm_sw_info_rcv.c +0 -0

84e7fe0..4381cbe 100644

opensm/osm_sw_info_rcv.c +1 -0

@@ -288,6 +288,7 @@ static boolean_t si_rcv_process_existing(IN
 osm_sm_t * sm,
 	ib_switch_info_t *p_si;
 	osm_si_context_t *p_si_context;
 	ib_smp_t *p_smp;
+	osm_epi_lft_change_event_t lft_change;
 	boolean_t is_change_detected = FALSE;
 

opensm/osm_sw_info_rcv.c +11 -0

 	OSM_LOG_ENTER(sm->p_log);

@@ -317,6 +318,17 @@ static boolean_t si_rcv_process_existing(IN
 osm_sm_t * sm,
 		}
 	}
 
+	if (sm->p_subn->first_time_master_sweep == FALSE &&
+	    p_si_context->set_method && p_si_context->lft_top_change) {
+		lft_change.p_sw = p_sw;
+		lft_change.flags = LFT_CHANGED_LFT_TOP;
+		lft_change.lft_top = cl_ntoh16(p_si->lin_top);
+		lft_change.block_num = 0;
+		osm_opensm_report_event(sm->p_subn->p_osm,
+					OSM_EVENT_ID_LFT_CHANGE,
+					&lft_change);
+	}
+
 	OSM_LOG_EXIT(sm->p_log);
 	return is_change_detected;

}

opensm/osm_ucast_mgr.c index renamed from opensm/osm_ucast_mgr.c +0 -0

7130ccf..12db434 100644

opensm/osm_ucast_mgr.c +0 -2

@@ -918,8 +918,6 @@ static void ucast_mgr_set_fwd_top(IN
 cl_map_item_t * p_map_item,
 
 	p_path =
 osm_physp_get_dr_path_ptr(osm_node_get_physp_ptr(p_node, 0));
 
-	p_sw->lft_change = 0;
-
 	/*

opensm/osm_ucast_mgr.c +3 -2

 	   Set the top of the unicast forwarding table.
 	 */

@@ -928,8 +926,9 @@ static void ucast_mgr_set_fwd_top(IN
 cl_map_item_t * p_map_item,
 	if (lin_top != si.lin_top) {
 		set_swinfo_require = TRUE;
 		si.lin_top = lin_top;
-		p_sw->lft_change = 1;
-	}
+		context.si_context.lft_top_change = TRUE;
+	} else
+		context.si_context.lft_top_change = FALSE;
 
 	/* check to see if the change state bit is on. If it is - then we

opensm/osm_ucast_mgr.c +1 -1

 	   need to clear it. */

@@ -966,7 +965,7 @@ static void ucast_mgr_set_fwd_top(IN
 cl_map_item_t * p_map_item,  }
 
 static int set_lft_block(IN osm_switch_t *p_sw, IN osm_ucast_mgr_t
 *p_mgr,
-			 IN uint16_t block_id_ho, IN unsigned last_block)
+			 IN uint16_t block_id_ho)
 {
 	uint8_t block[IB_SMP_DATA_SIZE];

opensm/osm_ucast_mgr.c +0 -2

 	osm_madw_context_t context;

@@ -996,8 +995,6 @@ static int set_lft_block(IN osm_switch_t *p_sw, IN
 osm_ucast_mgr_t *p_mgr,
 		     IB_SMP_DATA_SIZE)))
 		return 0;
 
-	p_sw->lft_change = 1;
-
 	OSM_LOG(p_mgr->p_log, OSM_LOG_DEBUG,
 		"Writing FT block %u to switch 0x%" PRIx64 "\n",

block_id_ho,

opensm/osm_ucast_mgr.c +0 -5

 		cl_ntoh64(context.lft_context.node_guid));

@@ -1008,11 +1005,6 @@ static int set_lft_block(IN osm_switch_t *p_sw, IN
 osm_ucast_mgr_t *p_mgr,
 			     cl_hton32(block_id_ho),
 			     CL_DISP_MSGID_NONE, &context);
 
-	if (!p_mgr->p_subn->first_time_master_sweep &&
-	    block_id_ho == last_block && p_sw->lft_change)
-		osm_opensm_report_event(p_mgr->p_subn->p_osm,
-					OSM_EVENT_ID_LFT_CHANGE,
 p_sw);
-
 	if (status != IB_SUCCESS) {

opensm/osm_ucast_mgr.c +0 -0

 		OSM_LOG(p_mgr->p_log, OSM_LOG_ERROR, "ERR 3A05: "
 			"Sending linear fwd. tbl. block failed (%s)\n", @@ -

1033,8 +1025,7 @@ static void ucast_mgr_pipeline_fwd_tbl(osm_ucast_mgr_t * p_mgr)

opensm/osm_ucast_mgr.c +1 -2

 	for (i = 0; i < max_block; i++)
 		for (item = cl_qmap_head(tbl); item != cl_qmap_end(tbl);
 		     item = cl_qmap_next(item))
-			set_lft_block((osm_switch_t *)item, p_mgr,
-				      i, max_block - 1);
+			set_lft_block((osm_switch_t *)item, p_mgr, i);
 }
 
 void osm_ucast_mgr_set_fwd_tables(osm_ucast_mgr_t * p_mgr) diff --git

a/osmeventplugin/src/osmeventplugin.c
b/osmeventplugin/src/osmeventplugin.c
index fed2bac..fc27c5a 100644

osmeventplugin/src/osmeventplugin.c +0 -0

@@ -156,11 +156,13 @@ static void handle_trap_event(_log_events_t *log,
 ib_mad_notice_attr_t *p_ntc)
 
 /**

==========================================================
===============
*/
-static void handle_lft_change_event(_log_events_t *log, osm_switch_t *p_sw)

osmeventplugin/src/osmeventplugin.c +3 -2

+static void handle_lft_change_event(_log_events_t *log,
+				    osm_epi_lft_change_event_t *lft_change)
 {
 	fprintf(log->log_file,
-		"LFT changed for switch 0x%" PRIx64 "\n",
-		cl_ntoh64(osm_node_get_node_guid(p_sw->p_node)));
+		"LFT changed for switch 0x%" PRIx64 " flags 0x%x LFTTop %u

block %d\n",

osmeventplugin/src/osmeventplugin.c +1 -0

+		cl_ntoh64(osm_node_get_node_guid(lft_change->p_sw-

Quoted 1 line

p_node)),

osmeventplugin/src/osmeventplugin.c +1 -0

+		lft_change->flags, lft_change->lft_top, lft_change-

Quoted 1 line

block_num);

}

/**
==========================================================
===============

osmeventplugin/src/osmeventplugin.c +2 -2

@@ -201,7 +203,7 @@ static void report(void *_log, osm_epi_event_id_t
 event_id, void *event_data)
 		fprintf(log->log_file, "SA DB dump file updated\n");
 		break;
 	case OSM_EVENT_ID_LFT_CHANGE:
-		handle_lft_change_event(log, (osm_switch_t *)
 event_data);
+		handle_lft_change_event(log,
 (osm_epi_lft_change_event_t *)
+event_data);
 		break;
 	case OSM_EVENT_ID_MAX:
 	default:
--

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

This changes the data sent by OSM_EVENT_ID_LFT_CHANGE.

I think this is a dangerous precedent.

Even though plugins must be coded to specific versions of OpenSM I don't think they will fail to compile with this change and users would get random behavior when they try to use the event_data passed.

I think it might be more appropriate to define a new event id say OSM_EVENT_ID_LFT_BLOCK_CHANGE.

Ira

-----Original Message-----
From: Hal Rosenstock [mailto:[email protected]]

Quoted 1 line · Ira

This changes the data sent by OSM_EVENT_ID_LFT_CHANGE.

That event was just added and was experimental rather than being some preexisting event already in some release.

Quoted 2 lines

I think this is a dangerous precedent.

It's not setting this precedent.

-- Hal

Quoted 1 line

Even though plugins must be coded to specific versions of OpenSM I don't

think they will fail to compile with this change and users would get random behavior when they try to use the event_data passed.

Quoted 2 lines

I think it might be more appropriate to define a new event id say

OSM_EVENT_ID_LFT_BLOCK_CHANGE.

Quoted 2 lines

Ira