[openib-general] [PATCH] osm: reviewing osmtest - osmt_multicast.c

[Hal Rosenstock]

On Sun, 2006-10-15 at 10:51, Yevgeny Kliteynik wrote:
> Hal Rosenstock wrote:
> > On Wed, 2006-10-11 at 06:23, Yevgeny Kliteynik wrote:
> >> Hi Hal
> >>
> >> Fixing a few problems in the multicast test flow,
> >> plus some cosmetics.
> >>
> >> Yevgeny
> >>
> >> Signed-off-by:  Yevgeny Kliteynik <kliteyn at dev.mellanox.co.il>
> > 
> > Thanks. Applied. See question below...
> > 
> >> Index: osmt_multicast.c
> >> ===================================================================
> >> --- osmt_multicast.c	(revision 9776)
> >> +++ osmt_multicast.c	(working copy)
> > 
> > [snip...]
> > 
> >> @@ -1808,14 +1865,54 @@ osmt_run_mcast_flow( IN osmtest_t * cons
> >>  
> >>    /*  o15.0.1.6: */
> >>    /*  - Create a new MCG with valid requested MGID. */
> >> +  osmt_init_mc_query_rec(p_osmt, &mc_req_rec);
> >> +  mc_req_rec.mgid = good_mgid;
> >>  
> >>    osm_log( &p_osmt->log, OSM_LOG_INFO,
> >>             "osmt_run_mcast_flow: "
> >> -           "Checking Create given MGID=0x%016" PRIx64 " : "
> >> +           "Checking Create given valid MGID=0x%016" PRIx64 " : "
> >> +           "0x%016" PRIx64 " (o15.0.1.6)...\n",
> >> +           cl_ntoh64(mc_req_rec.mgid.unicast.prefix),
> >> +           cl_ntoh64(mc_req_rec.mgid.unicast.interface_id));
> >> +
> >> +  /* Before creation, need to check that this group doesn't exist */
> >> +  osm_log( &p_osmt->log, OSM_LOG_INFO,
> >> +           "osmt_run_mcast_flow: "
> >> +           "Verifying that MCGroup with this MGID doesn't exist by trying to Join it (o15.0.1.13)...\n");
> >> +
> >> +  ib_member_set_join_state(&mc_req_rec, IB_MC_REC_STATE_NON_MEMBER);
> >> +
> >> +  osm_log( &p_osmt->log, OSM_LOG_ERROR,
> >> +           "osmt_run_mcast_flow: " EXPECTING_ERRORS_START "\n" );
> >> +  status = osmt_send_mcast_request( p_osmt, 1, /*  join */
> >> +                                    &mc_req_rec,
> >> +                                    comp_mask,
> >> +                                    &res_sa_mad );
> >> +  osm_log( &p_osmt->log, OSM_LOG_ERROR,
> >> +           "osmt_run_mcast_flow: " EXPECTING_ERRORS_END "\n" );
> >> +
> >> +  if ((status != IB_REMOTE_ERROR) ||
> >> +      (res_sa_mad.status != IB_SA_MAD_STATUS_REQ_INVALID))
> >> +  {
> >> +    osm_log( &p_osmt->log, OSM_LOG_ERROR,
> >> +             "osmt_run_mcast_flow: ERR 0301: "
> >> +             "Tried joining group that shouldn't have existed - got %s/%s\n",
> >> +             ib_get_err_str( status ),
> >> +             ib_get_mad_status_str( (ib_mad_t*)(&res_sa_mad) )
> >> +             );
> >> +    status = IB_ERROR;
> >> +    goto Exit;
> >> +  }
> > 
> > In the event this works, the SA is potentially left with some bad state
> > because of this. Should the join be removed for this case ?
> 
> When osmtest completes w/o error, at the end of the test flow
> SM is returned to its initial state.

Right.

> When osmtest runs into some error, it exits, leaving SM as is.
> I think it's better to leave SM state as is after discovering 
> a failure for further examination.

I think this should depend on the failure.

> But even if we wanted to 'clean up', it is not always possible.
> For instance, what happens when multicast group was created by 
> osmtest, and the group removal fails?

Right but the ones which can be cleaned up should be IMO.

-- Hal

> 
> --
> Yevgeny
> > 
> > -- Hal
> >