On Thu, 2007-03-29 at 13:16, Suresh Shelvapille wrote: > > -----Original Message----- > > From: Hal Rosenstock [mailto:[EMAIL PROTECTED] > > Sent: Thursday, March 29, 2007 2:47 PM > > To: Roland Dreier > > Cc: [email protected]; Sean Hefty; Suresh Shelvapille > > Subject: Re: [PATCH] IB/core: Enhance SMI for switch support > > > > On Thu, 2007-03-29 at 12:12, Roland Dreier wrote: > > > > + retsmi = smi_check_forward_dr_smp(&recv->mad.smp); > > > > + if (!retsmi) > > > > goto local; > > > > - if (!smi_handle_dr_smp_send(&recv->mad.smp, > > > > - > > > port_priv->device->node_type, > > > > - port_priv->port_num)) > > > > - goto out; > > > > - if (!smi_check_local_smp(&recv->mad.smp, > > > port_priv->device)) > > > > + > > > > + if (retsmi == 1) { /* don't forward */ > > > > > > > /* > > > > * Return 1 if the received DR SMP should be forwarded to the send > > > queue > > > > * Return 0 if the SMP should be completed up the stack > > > > + * Return 2 if the SMP should be forwarded (for switches only) > > > > */ > > > > int smi_check_forward_dr_smp(struct ib_smp *smp) > > > > > > I think this has now crossed the line where these magic return values > > > should be named enums instead. > > > > OK; I'll make these into enums. > > > > > Especially the "if (!retsmi)" is very > > > hard to follow. > > > > Is it hard to follow ? > > > > > > +/* > > > > + * Return the forwarding port number from initial_path for outgoing > > > SMP and > > > > + * from return_path for returning SMP > > > > + */ > > > > +static inline int smi_get_fwd_port(struct ib_smp *smp) > > > > +{ > > > > + return (!ib_get_smp_direction(smp) ? > > > smp->initial_path[smp->hop_ptr+1] : > > > > + smp->return_path[smp->hop_ptr-1]); > > > > +} > > > > > > This has exactly one caller. I would just put this function in the .c > > > file where it's called. > > > > I'll resubmit a v2 of this patch later with this change and the enum > > change. > > > > the reason this was made into a function and put inside the header file was > because > paths weren't accessed directly within mad.c. > > If you are going to do what Roland is suggesting, then why have a function? > Why not > just stick it in-line as I had before.
Because this is a SMI function and touches the DR path. Nothing in mad.c currently directly touches the DR paths without using a SMI function. -- Hal > Thanks, > Suri > _______________________________________________ general mailing list [email protected] http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
