Hello!
Quoting r. Roland Dreier ([EMAIL PROTECTED]) "Re: [openib-general] [PATCH] fill 
in missing parameters for MAD_IFC":
>     Michael> I dont want to go look up the type of structures in
>     Michael> ib_verbs.h, which do not have to match hardware exactly
>     Michael> after all.
> 
> Yeah, I had avoided putting direct dependencies on ib_verbs.h into
> mthca_cmd.c, but I didn't see a nice way to avoid it for the extended
> MAD info passed to MAD_IFC.

Well, the dependency will be there, but I think relying on the
field width in ib_verbs.h matching the hardware is a bit on the ugly side.

>     Michael> Maybe not just now, but what about eventually making
>     Michael> mthca_cmd.c use structures along the lines of mthca_cq.c?
> 
> The thing I don't like about doing that is that things like
> sl_g_mlpath end up being pretty ugly to handle.

It does not have to be that way, I put sl and path in the same
word because they are that way in the cqe, but they could be split.

+               val = in_wc->sl << 4;
+               MTHCA_PUT(box, val,               MAD_IFC_SL_OFFSET);
+
+               val = in_wc->dlid_path_bits |
+                       (in_wc->wc_flags & IB_WC_GRH ? 0x80 : 0);
+               MTHCA_PUT(box, val,               MAD_IFC_GRH_OFFSET);

Becomes:

+               info->sl = in_wc->sl << 4;
+               info->sl_g_mlpath = cpu_to_be16(in_wc->dlid_path_bits |
+                       (in_wc->wc_flags & IB_WC_GRH ? 0x80 : 0);

Convinced?

>     Michael> The check seems to be in there. Its not that important
>     Michael> either way.
> 
> I did misread the BUG_ON -- but it does seem there are cases where it
> oopses without really needing to.  Rather than a BUG_ON, how about
> just moving the code to make sure we don't check without in_wc into
> the actual MAD_IFC routine:
> 
>       /*
>        * Key check traps can't be generated unless we have in_wc to
>        * tell us where to send the trap.
>        */
>       if (ignore_mkey || !in_wc)
>               op_modifier |= 0x1;
>       if (ignore_bkey || !in_wc)
>               op_modifier |= 0x2;

Fine with me.

> Here's my current patch:

OK.

MST
_______________________________________________
openib-general mailing list
[email protected]
http://openib.org/mailman/listinfo/openib-general

To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general

Reply via email to