On Mon, Nov 2, 2009 at 9:00 PM, Sasha Khapyorsky <[email protected]> wrote: > On 06:50 Mon 02 Nov , Hal Rosenstock wrote: >> >> Move some code out into separate handle_babbling_port routine >> for readability/maintainability >> >> Signed-off-by: Hal Rosenstock <[email protected]> >> --- >> diff --git a/opensm/opensm/osm_trap_rcv.c b/opensm/opensm/osm_trap_rcv.c >> index 5790461..1621fbc 100644 >> --- a/opensm/opensm/osm_trap_rcv.c >> +++ b/opensm/opensm/osm_trap_rcv.c >> @@ -312,6 +312,61 @@ static void log_trap_info(osm_log_t *p_log, >> ib_mad_notice_attr_t *p_ntci, >> cl_ntoh16(source_lid), cl_ntoh64(trans_id)); >> } >> >> +static int handle_babbling_port(osm_sm_t *sm, ib_mad_notice_attr_t *p_ntci, >> + uint32_t num_received, >> + boolean_t physp_change_trap, >> + osm_physp_t **pp_physp, >> + boolean_t *run_heavy_sweep, >> + uint64_t *event_wheel_timeout) >> +{ >> + if (print_num_received(num_received)) >> + OSM_LOG(sm->p_log, OSM_LOG_ERROR, "ERR 3804: " >> + "Received trap %u times consecutively\n", >> + num_received); >> + /* If the trap provides info about a bad port, mark it as unhealthy. */ >> + if (physp_change_trap == TRUE) { >> + /* get the port */ >> + *pp_physp = get_physp_by_lid_and_num(sm, >> + >> cl_ntoh16(p_ntci->data_details.ntc_129_131.lid), >> + >> p_ntci->data_details.ntc_129_131.port_num); >> + >> + if (!*pp_physp) >> + OSM_LOG(sm->p_log, OSM_LOG_ERROR, "ERR 3805: " >> + "Failed to find physical port by lid:%u >> num:%u\n", >> + >> cl_ntoh16(p_ntci->data_details.ntc_129_131.lid), >> + p_ntci->data_details.ntc_129_131.port_num); >> + else { >> + /* When babbling port policy option is enabled and >> + Threshold for disabling a "babbling" port is >> exceeded */ >> + if (sm->p_subn->opt.babbling_port_policy && >> + num_received >= 250 && >> + disable_port(sm, *pp_physp) == 0) >> + return 1; >> + >> + OSM_LOG(sm->p_log, OSM_LOG_VERBOSE, >> + "Marking unhealthy physical port by lid:%u >> num:%u\n", >> + >> cl_ntoh16(p_ntci->data_details.ntc_129_131.lid), >> + p_ntci->data_details.ntc_129_131.port_num); >> + /* check if the current state of the p_physp is >> healthy. If >> + it is - then this is a first change of state. Run a >> heavy sweep. >> + if it is not - no need to mark it again - just >> restart the timer. */ >> + if (osm_physp_is_healthy(*pp_physp)) { >> + osm_physp_set_health(*pp_physp, FALSE); >> + /* Make sure we sweep again - force a heavy >> sweep. */ >> + /* The sweep should be done only after the >> re-registration, or >> + else we'll be losing track of the timer. */ >> + *run_heavy_sweep = TRUE; >> + } >> + /* If we are marking the port as unhealthy - we want to >> + keep this for a longer period of time than the >> + OSM_DEFAULT_TRAP_SUPRESSION_TIMEOUT. Use the >> + OSM_DEFAULT_UNHEALTHY_TIMEOUT */ >> + *event_wheel_timeout = OSM_DEFAULT_UNHEALTHY_TIMEOUT; >> + } >> + return 0; >> +} >> + > > It looks that you are moving the code as it is to a separate function > where many flow switchers (run_heavy_sweep, event_wheel_timeout, > pp_physp) are magically modified. > > What about something like below instead?
Reviewed-by: Hal Rosenstock <[email protected]> > > Sasha > > > >From 5ed9114a32f9536123af4bd135c7cc603d340f92 Mon Sep 17 00:00:00 2001 > From: Sasha Khapyorsky <[email protected]> > Date: Tue, 3 Nov 2009 03:46:54 +0200 > Subject: [PATCH] opensm/osm_trap_rcv.c: some consolidation > > Consolidate noisy port handling code in shutup_noisy_port() function. > As before it will try to disable port or to mark it unhealthy. > > Signed-off-by: Sasha Khapyorsky <[email protected]> > --- > opensm/opensm/osm_trap_rcv.c | 113 > ++++++++++++++++++++---------------------- > 1 files changed, 53 insertions(+), 60 deletions(-) > > diff --git a/opensm/opensm/osm_trap_rcv.c b/opensm/opensm/osm_trap_rcv.c > index 0ee4e77..dae892e 100644 > --- a/opensm/opensm/osm_trap_rcv.c > +++ b/opensm/opensm/osm_trap_rcv.c > @@ -226,7 +226,6 @@ static int disable_port(osm_sm_t *sm, osm_physp_t *p) > uint8_t payload[IB_SMP_DATA_SIZE]; > osm_madw_context_t context; > ib_port_info_t *pi = (ib_port_info_t *)payload; > - int ret; > > /* select the nearest port to master opensm */ > if (p->p_remote_physp && > @@ -236,10 +235,6 @@ static int disable_port(osm_sm_t *sm, osm_physp_t *p) > /* If trap 131, might want to disable peer port if available */ > /* but peer port has been observed not to respond to SM requests */ > > - OSM_LOG(sm->p_log, OSM_LOG_ERROR, "ERR 3810: " > - "Disabling physical port 0x%016" PRIx64 " num:%u\n", > - cl_ntoh64(osm_physp_get_port_guid(p)), p->port_num); > - > memcpy(payload, &p->port_info, sizeof(ib_port_info_t)); > > /* Set port to disabled/down */ > @@ -253,15 +248,10 @@ static int disable_port(osm_sm_t *sm, osm_physp_t *p) > context.pi_context.light_sweep = FALSE; > context.pi_context.active_transition = FALSE; > > - ret = osm_req_set(sm, osm_physp_get_dr_path_ptr(p), > - payload, sizeof(payload), IB_MAD_ATTR_PORT_INFO, > - cl_hton32(osm_physp_get_port_num(p)), > - CL_DISP_MSGID_NONE, &context); > - if (ret) > - OSM_LOG(sm->p_log, OSM_LOG_ERROR, "ERR 3811: " > - "Request to set PortInfo failed\n"); > - > - return ret; > + return osm_req_set(sm, osm_physp_get_dr_path_ptr(p), > + payload, sizeof(payload), IB_MAD_ATTR_PORT_INFO, > + cl_hton32(osm_physp_get_port_num(p)), > + CL_DISP_MSGID_NONE, &context); > } > > static void log_trap_info(osm_log_t *p_log, ib_mad_notice_attr_t *p_ntci, > @@ -301,6 +291,42 @@ static void log_trap_info(osm_log_t *p_log, > ib_mad_notice_attr_t *p_ntci, > cl_ntoh16(source_lid), cl_ntoh64(trans_id)); > } > > +static int shutup_noisy_port(osm_sm_t *sm, uint16_t lid, uint8_t port, > + unsigned num) > +{ > + osm_physp_t *p = get_physp_by_lid_and_num(sm, lid, port); > + if (!p) { > + OSM_LOG(sm->p_log, OSM_LOG_ERROR, "ERR 3805: " > + "Failed to find physical port by lid:%u num:%u\n", > + lid, port); > + return -1; > + } > + > + /* When babbling port policy option is enabled and > + Threshold for disabling a "babbling" port is exceeded */ > + if (sm->p_subn->opt.babbling_port_policy && num >= 250) { > + OSM_LOG(sm->p_log, OSM_LOG_VERBOSE, > + "Disabling noisy physical port 0x%016" PRIx64 > + ": lid %u, num %u\n", > + cl_ntoh64(osm_physp_get_port_guid(p)), lid, port); > + if (disable_port(sm, p)) > + OSM_LOG(sm->p_log, OSM_LOG_ERROR, "ERR 3811: " > + "Failed to disable.\n"); Might want to include port information in this log message. -- Hal > + else > + return 1; > + } > + > + /* check if the current state of the p_physp is healthy. If > + it is - then this is a first change of state. Run a heavy sweep. */ > + if (osm_physp_is_healthy(p)) { > + OSM_LOG(sm->p_log, OSM_LOG_VERBOSE, > + "Marking unhealthy physical port by lid:%u num:%u\n", > + lid, port); > + osm_physp_set_health(p, FALSE); > + return 2; > + } > + return 0; > +} > /********************************************************************** > **********************************************************************/ > static void trap_rcv_process_request(IN osm_sm_t * sm, > @@ -438,7 +464,7 @@ static void trap_rcv_process_request(IN osm_sm_t * sm, > /* Now we know how many times it provided this trap */ > if (num_received > 10) { > if (print_num_received(num_received)) > - OSM_LOG(sm->p_log, OSM_LOG_ERROR, "ERR 3804: " > + OSM_LOG(sm->p_log, OSM_LOG_VERBOSE, > "Received trap %u times > consecutively\n", > num_received); > /* > @@ -446,49 +472,17 @@ static void trap_rcv_process_request(IN osm_sm_t * sm, > * we mark it as unhealthy. > */ > if (physp_change_trap == TRUE) { > - /* get the port */ > - p_physp = get_physp_by_lid_and_num(sm, > - cl_ntoh16 > - > (source_lid), > - port_num); > - > - if (!p_physp) > - OSM_LOG(sm->p_log, OSM_LOG_ERROR, > - "ERR 3805: " > - "Failed to find physical port > by lid:%u num:%u\n", > - cl_ntoh16(source_lid), > - port_num); > - else { > - /* When babbling port policy option > is enabled and > - Threshold for disabling a > "babbling" port is exceeded */ > - if (sm->p_subn->opt. > - babbling_port_policy > - && num_received >= 250 > - && disable_port(sm, p_physp) == 0) > - goto Exit; > - > - OSM_LOG(sm->p_log, OSM_LOG_VERBOSE, > - "Marking unhealthy physical > port by lid:%u num:%u\n", > - cl_ntoh16(source_lid), > - port_num); > - /* check if the current state of the > p_physp is healthy. If > - it is - then this is a first > change of state. Run a heavy sweep. > - if it is not - no need to mark it > again - just restart the timer. */ > - if (osm_physp_is_healthy(p_physp)) { > - osm_physp_set_health(p_physp, > - FALSE); > - /* Make sure we sweep again - > force a heavy sweep. */ > - /* The sweep should be done > only after the re-registration, or > - else we'll be losing track > of the timer. */ > - run_heavy_sweep = TRUE; > - } > - /* If we are marking the port as > unhealthy - we want to > - keep this for a longer period of > time than the > - > OSM_DEFAULT_TRAP_SUPRESSION_TIMEOUT. Use the > - OSM_DEFAULT_UNHEALTHY_TIMEOUT */ > - event_wheel_timeout = > - OSM_DEFAULT_UNHEALTHY_TIMEOUT; > - } > + int ret = shutup_noisy_port(sm, > + > cl_ntoh16(source_lid), > + port_num, > + num_received); > + if (ret == 1) /* port disabled */ > + goto Exit; > + else if (ret == 2) /* unhealthy - run sweep */ > + run_heavy_sweep = TRUE; > + /* in any case increase timeout interval */ > + event_wheel_timeout = > + OSM_DEFAULT_UNHEALTHY_TIMEOUT; > } > } > > @@ -508,8 +502,7 @@ static void trap_rcv_process_request(IN osm_sm_t * sm, > if (num_received > 10 && run_heavy_sweep == FALSE) { > if (print_num_received(num_received)) > OSM_LOG(sm->p_log, OSM_LOG_VERBOSE, > - "Continuously received this trap %u > times. Ignoring\n", > - num_received); > + "Ignoring noisy traps.\n"); > goto Exit; > } > } > -- > 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 >
