Hefty, Sean wrote:
Other than that one nit, the other patches in this series look good to me. See below for comments on this one.
+struct ib_perf { + u8 base_version; + u8 mgmt_class; + u8 class_version; + u8 method; + __be16 status; + __be16 unused; + __be64 tid; + __be16 attr_id; + __be16 resv; + __be32 attr_mod; + u8 reserved[40]; + u8 data[192]; +} __attribute__ ((packed));
This should use struct ib_mad_hdr, possibly adding appropriate values to ib_mad.h for the 40 and 192. See the enum in ib_mad.h with IB_MGMT_* values. I would also rename this to struct ib_pm_mad to better match the existing mad definitions.
okay, replaced all the fields except for the last two with struct ib_mad_hdr and renamed the structure to ib_pma_mad (long live sed)
+struct ib_pma_classportinfo {This needs to be replaced with struct ib_class_port_info in ib_mad.h.
good catch, done
+struct ib_pma_portsamplescontrol { + u8 counter_width; /* only lower 3 bits */ + __be32 counter_mask0_9; /* 2, 10 * 3, bits */ + __be16 counter_mask10_14; /* 1, 5 * 3, bits */
Comments for the bit fields aren't very clear, maybe say something more like '10 3-bit fields'. For things like counter_width, my preference is something like /* resv: 7:3, counter width: 2:0 */ to clarify which bits are used and indicate that the others are currently reserved.
done, changed the comments as you suggested
+ __be16 counter_select[15]; +} __attribute__ ((packed));
There are fields missing at the end: reserved 32-bits, SamplesOnlyOptionMask 64-bit, reserved 896-bits. This should make the structure naturally aligned, which could remove the packed attribute.
done, added reserved fields and removed packing.
+struct ib_pma_portsamplesresult {this is naturally aligned
OKAY, removed packing here
+struct ib_pma_portsamplesresult_ext {this is naturally aligned
OKAY, removed packing also here
+struct ib_pma_portcounters { + u8 lli_ebor_errors; /* 4, 4, bits */
lli_ebor? what about link_overrun_errors instead, or at least something a little more descriptive? Enhancing the comment can also help: /* LocalLink: 7:4, BufferOverrun: 3:0 */
done, changed field name to link_overrun_errors and added the comment you suggested.
+ __be32 port_xmit_data; + __be32 port_rcv_data; + __be32 port_xmit_packets; + __be32 port_rcv_packets; +} __attribute__ ((packed));
missing PortXmitWait 32-bits at end
added, thanks
Rather than having IB_PMA_SEL_* and IB_PMA_SELX_*, we could follow what's done in ib_sa.h: IB_PM_<structure>_<field>
I would need some guidance here... maybe you can elaborate a little further with the V1 posting which will follow soon
Or. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
