[ofw] RE: IPoIB partitioning support potential bug?

[Chas Williams (CONTRACTOR)]

is your test non-default pkey a palindrome?

it looks like the pkey_array generated by __prepare_pKey_array() is in
host endian:

bus_driver.c:                   g_pkeys.pkey_num = __prepare_pKey_array(&pkeyString, (uint16_t*)g_pkeys.pkey_array);

later this is just assigned to the port_guid.pkey:

bus_port_mgr.c:                 p_port_ext->port_guid.pkey = g_pkeys.pkey_array[num_pdo -1];

port_guid.pkey has to be network endian right?

In message <39C75744D164D948A170E9792AF8E7CAACDEB7 at exil.voltaire.com>,"Slava Strebkov" writes:
>I applied the patch.
>Meanwhile it works without problems
>
>Slava
>
>
>-----Original Message-----
>From: Leonid Keller [mailto:leonid at mellanox.co.il] 
>Sent: Wednesday, June 04, 2008 8:28 PM
>To: Fab Tillier; Sean Hefty; Slava Strebkov; ofw at lists.openfabrics.org
>Subject: RE: [ofw] RE: IPoIB partitioning support potential bug?
>
>Please, review the attached patch. 
>
>> -----Original Message-----
>> From: Fab Tillier [mailto:ftillier at windows.microsoft.com] 
>> Sent: Tuesday, June 03, 2008 7:24 PM
>> To: Leonid Keller; Sean Hefty; Slava Strebkov; 
>> ofw at lists.openfabrics.org
>> Subject: RE: [ofw] RE: IPoIB partitioning support potential bug?
>> 
>> The driver should be fixed, and IPoIB too.
>> 
>> Note that all places where you take a pkey in the driver 
>> needs to expect it in network order.  I believe this applies 
>> to UD send work requests.
>> 
>> -Fab
>> 
>> > -----Original Message-----
>> > From: Leonid Keller [mailto:leonid at mellanox.co.il]
>> > Sent: Tuesday, June 03, 2008 3:27 AM
>> > To: Sean Hefty; Fab Tillier; Slava Strebkov; 
>> ofw at lists.openfabrics.org
>> > Subject: RE: [ofw] RE: IPoIB partitioning support potential bug?
>> >
>> > Yes, the drivers return pkeys in the host order.
>> > Is it safe to fix that ?
>> > What should be changed to adjust to that fix ?
>> >
>> >
>> > Index: mlx4/kernel/bus/ib/main.c
>> > ===================================================================
>> > --- mlx4/kernel/bus/ib/main.c   (revision 1222)
>> > +++ mlx4/kernel/bus/ib/main.c   (working copy)
>> > @@ -230,7 +230,7 @@
>> >         if (err)
>> >                 goto out;
>> >
>> > -       *pkey = be16_to_cpu(((__be16 *) out_mad->data)[index % 32]);
>> > +       *pkey = ((__be16 *) out_mad->data)[index % 32];
>> >
>> >  out:
>> >         kfree(in_mad);
>> > Index: mthca/kernel/mthca_provider.c
>> > ===================================================================
>> > --- mthca/kernel/mthca_provider.c       (revision 1222)
>> > +++ mthca/kernel/mthca_provider.c       (working copy)
>> > @@ -264,7 +264,7 @@
>> >                 int i;
>> >                 __be16 *pkey_chunk = (__be16 *)out_mad->data;
>> >                 for (i=0; i<32; ++i)
>> > -                       pkey[i] = cl_ntoh16(pkey_chunk[i]);
>> > +                       pkey[i] = pkey_chunk[i];
>> >         }
>> >
>> >   out:
>> >
>> > > -----Original Message-----
>> > > From: ofw-bounces at lists.openfabrics.org 
>> > > [mailto:ofw-bounces at lists.openfabrics.org] On Behalf Of Sean Hefty
>> > > Sent: Monday, June 02, 2008 8:57 PM
>> > > To: 'Fab Tillier'; Slava Strebkov; ofw at lists.openfabrics.org
>> > > Subject: RE: [ofw] RE: IPoIB partitioning support potential bug?
>> > >
>> > > >Ok, so does this mean that the pkey value was returned 
>> by the HCA 
>> > > >driver in host-order?  If so, this is a bug in the HCA driver.
>> > >
>> > > I agree with Fab here.  Looking at the code, it looks wrong.
>> > >
>> > > >From __port_create_bcast() and __port_get_bcast():
>> > >
>> > >       mcast_req.member_rec.mgid.raw[4] = (uint8_t)
>> > >               p_port->p_adapter->guids.port_guid.pkey;
>> > >       mcast_req.member_rec.mgid.raw[5] = (uint8_t)
>> > >               (p_port->p_adapter->guids.port_guid.pkey >> 8);
>> > >       mcast_req.member_rec.pkey =
>> > >               p_port->p_adapter->guids.port_guid.pkey;
>> > >
>> > > The pkay is swapped in one case, but not the other.
>> > >
>> > > The pkey in the mcmember record is correct, but the 
>> created mgid is 
>> > > wrong.  The pkey for the group itself is correct, so I 
>> don't think 
>> > > the switches or SM would detect the problem.
>> > > But the mgid won't be the same.  Has anyone tried to use this 
>> > > support mixing windows and linux?  I'm guessing that's 
>> when we would 
>> > > see the problem.
>> > >
>> > > - Sean
>> > >
>> > > _______________________________________________
>> > > ofw mailing list
>> > > ofw at lists.openfabrics.org
>> > > http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ofw
>> > >
>> 
>_______________________________________________
>ofw mailing list
>ofw at lists.openfabrics.org
>http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ofw
>
>