[openib-general] RE: [PATCH] OpenSM - check port capabilities to create MC group

[Yael Kalka]

Hi Sasha,
You are right. I will send a fix for this.
Thanks,
Yael

> -----Original Message-----
> From: Sasha Khapyorsky [mailto:sashak at voltaire.com]
> Sent: Sunday, March 05, 2006 2:55 PM
> To: Yael Kalka
> Cc: halr at voltaire.com; openib-general at openib.org; Eitan Zahavi; Ofer
Gigi
> Subject: Re: [PATCH] OpenSM - check port capabilities to create MC
group
> 
> Hello Yael,
> 
> On 11:26 Sun 05 Mar     , Yael Kalka wrote:
> >
> > After checking in Sasha's patch regarding the port capabilities for
> > joining MC group, and doing some testing here, we've noticed the
> > following problem:
> > Currently, when requesting to create MC group with MTU(rate)
selector
> > 1 (meaning less than rate specified), the MC group is created with
> > MTU(rate) requested - 1. This is without checking the MTU(rate) of
the
> > port requesting the creation of the multicast group.
> > This means that if, for example, port with MTU=2 sends a request for
> > MC group creation with MTU selector=1 and MTU=5, Opensm will try to
> > create a MC group with MTU=4, and fail, since the port capabilities
> > are not realizable.
> > The following patch adds creation of the MC group with MTU(rate)
that
> > also takes into account the MTU(rate) of the port requesting the
> creation.
> 
> The idea looks reasonable for me. Some note about patch however
(below).
> 
> > @@ -1092,10 +1097,17 @@ __mgrp_request_is_realizable(
> >        }
> >        break;
> >      case 1: /* Less than MTU specified */
> > -      /* if the requested MTU is not already the minimal we just
> > -         use one lower */
> > +      /* if the requested MTU is not already the minimal, then we
will
> > +         use the smaller of the two:
> > +         a. one lower then the required
> > +         b. the mtu of the requesting port
> > +         If the p_pi is NULL - this means there is no requestor
port,
> then
> > +         just use mtu one lower then the required. */
> >        if ( mtu_required > 1 )
> >        {
> > +        if (p_pi && ib_port_info_get_mtu_cap(p_pi) < (mtu_required
-
> 1))
> > +          p_mcm_rec->mtu = ib_port_info_get_mtu_cap(p_pi);
>                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^
> Does this will overwrite mtu_sel bits in p_mcm_rec->mtu?
> 
> > @@ -1161,10 +1173,18 @@ __mgrp_request_is_realizable(
> >        }
> >        break;
> >      case 1: /* Less than RATE specified */
> > -      /* if the requested RATE is not already the minimal we just
> > -         use one lower */
> > +      /* if the requested RATE is not already the minimal, then we
will
> > +         use the smaller of the two:
> > +         a. one lower then the required
> > +         b. the rate of the requesting port
> > +         If the p_pi is NULL - this means there is no requestor
port,
> then
> > +         just use rate one lower then the required. */
> > +
> >        if ( rate_required > 2 )
> >        {
> > +        if (p_pi && ib_port_info_compute_rate(p_pi) <
(rate_required -
> 1))
> > +          p_mcm_rec->rate = ib_port_info_compute_rate(p_pi);
> 
> Ditto.
> 
> Thanks,
> Sasha.