[openib-general] [PATCH] fill in missing parameters for MAD_IFC
Michael S. Tsirkin
mst at mellanox.co.il
Mon Jan 17 00:43:21 PST 2005
Hello, Roland!
Quoting Roland Dreier
> I reworked this to clean a few things up and match the style of
> mthca_cmd.c a little better.
This is a valid approach. Its your call.
Generally, there is one thing I dislike in the MTHCA_PUT macro -
specifically, that I have to look up the size of the *source* variable
to know which conversion is made. Ideally its the destination
that should dictate the right size.
Consider this:
MTHCA_PUT(box, in_wc->slid, MAD_IFC_RLID_OFFSET);
So in_wc->slid happends to have size 16 bit, and so it fits exactly
the inbox format. Its not just luck, but when I am trying to grok
the specific hardware command, I dont want to go look up the type of
structures in ib_verbs.h, which do not have to match hardware exactly
after all.
With the inbox described by a structure, I can see the correct conversion
is being applied at a glance, without going to another file.
Further, I also think that specifying offsets
by means of a structure has an advantage that it is easier to see
that you did not forget to fill in some field.
Maybe not just now, but what about eventually making mthca_cmd.c
use structures along the lines of mthca_cq.c ?
> Does this still seem OK?
Yes, it looks allright to commit.
> (I got rid of the BUG_ON because I think the test
>
> BUG_ON((!ignore_bkey && !ignore_mkey) && !in_wc);
>
> is not really correct -- eg passing a MAD known not to be BM-class
> with only ignore_mkey set and no in_wc would be fine -- and I
> couldn't decide on a check that did seem worth oopsing for)
The check seems to be in there. Its not that important either way.
> + BUG_ON((!ignore_bkey && !ignore_mkey) && !in_wc);
> +
MST
More information about the general
mailing list