>+static int data_offset(u8 mgmt_class)
>+{
>+ if (mgmt_class == IB_MGMT_CLASS_SUBN_ADM)
>+ return IB_MGMT_SA_HDR;
>+ else if ((mgmt_class >= IB_MGMT_CLASS_VENDOR_RANGE2_START) &&
>+ (mgmt_class <= IB_MGMT_CLASS_VENDOR_RANGE2_END))
>+ return IB_MGMT_VENDOR_HDR;
>+ else
>+ return IB_MGMT_RMPP_HDR;
>+}
I think that the RMPP code may have this same routine. If so, maybe we can make
this an inline function.
>+static int copy_recv_mad(struct ib_mad_recv_wc *mad_recv_wc,
>+ struct ib_umad_packet *packet)
>+{
>+ struct ib_mad_recv_buf *seg_buf;
>+ struct ib_rmpp_mad *rmpp_mad;
>+ void *data;
>+ struct ib_mad_multipacket_seg *seg;
>+ int size, len, offset;
>+ u8 flags;
>+
>+ len = mad_recv_wc->mad_len;
>+ if (len <= sizeof(struct ib_mad)) {
>+ memcpy(&packet->mad.data, mad_recv_wc->recv_buf.mad, len);
>+ return 0;
>+ }
>+
>+ /* Multipacket (RMPP) MAD */
>+ offset = data_offset(mad_recv_wc->recv_buf.mad->mad_hdr.mgmt_class);
>+
>+ list_for_each_entry(seg_buf, &mad_recv_wc->rmpp_list, list) {
>+ rmpp_mad = (struct ib_rmpp_mad *)seg_buf->mad;
>+ flags = ib_get_rmpp_flags(&rmpp_mad->rmpp_hdr);
>+
>+ if (flags & IB_MGMT_RMPP_FLAG_FIRST) {
>+ size = sizeof(*rmpp_mad);
>+ memcpy(&packet->mad.data, rmpp_mad, size);
>+ } else {
>+ data = (void *) rmpp_mad + offset;
>+ if (flags & IB_MGMT_RMPP_FLAG_LAST)
>+ size = len;
>+ else
>+ size = sizeof(*rmpp_mad) - offset;
>+ seg = kmalloc(sizeof(struct ib_mad_multipacket_seg) +
>+ sizeof(struct ib_rmpp_mad) - offset,
>+ GFP_KERNEL);
>+ if (!seg)
>+ return -ENOMEM;
>+ memcpy(seg->data, data, size);
>+ list_add_tail(&seg->list, &packet->seg_list);
>+ }
>+ len -= size;
>+ }
>+ return 0;
>+}
It would be more efficient to just queue the received MAD until it can be copied
directly to the userspace buffer, rather than copying it into a temporary
buffer.
>+
>+static struct ib_umad_packet *alloc_packet(void)
>+{
>+ struct ib_umad_packet *packet;
>+ int length = sizeof *packet + sizeof(struct ib_mad);
>+
>+ packet = kzalloc(length, GFP_KERNEL);
>+ if (!packet) {
>+ printk(KERN_ERR "alloc_packet: mem alloc failed for length
%d\n",
>+ length);
>+ return NULL;
>+ }
>+ INIT_LIST_HEAD(&packet->seg_list);
>+ return packet;
>+}
We should probably just drop this function. It looks like it's only called in
one place, plus would only save a single line of code for each place that it is
called.
- Sean
_______________________________________________
openib-general mailing list
[email protected]
http://openib.org/mailman/listinfo/openib-general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
