On Thu, 2007-02-01 at 20:48 -0800, Roland Dreier wrote: > > Have you had a chance to review this? > > Still on my list. > > Can we trade? Can you look at the IPoIB connected mode stuff in the > ipoib-cm branch in > > git://git.kernel.org/pub/scm/linux/kernel/git/roland/infiniband.git > > and let me know if you see anything you don't like? > > - R.
Here are my comments. I'm not an ib cm expert though. These are mostly questions: Since IPoIB is using IP addresses already, wouldn't it be simpler to use the rdma cm to setup connections? Could you optimize this design and only signal some of the tx wrs? In ipoib_cm_send() you call ipoib_cm_skb_too_long() if the packet is too large for the interface mtu. And you print a warning. But ipoib_cm_skb_too_long() actually queues the packet for the cm case. For ud it just drops the packet. The skb task for cm then will send a ICMP_DEST_UNREACH for these packets. Why the difference? Also if this packet came from the local stack via a local application, you don't want to send DEST_UNREACH, right? (I'm probably just confused about the purpose of this). In ipoib_cm_tx_completion() you rearm, then drain the cq. I thought there was some reason that it was better to do drain/rearm/drain? Something about if you rearm and there's a cq entry mthca does another immediate interrupt? In ipoib_cm_handle_tx_wc(): When can a tx completion happen with a wr_id that isn't within the ipoib_sendq_size range? This looks like it is really a bug condition that should never happen. I see the same code in the rx completion path too. Also, what's up with the /* FIXME */ comment? You lock the priv->lock inside of the priv->tx_lock. Is this ordering correct and consistent across all the code? ipoib_cm_handle_rx_wc() - what's up with the XXX comment? What's the algorithm to keep enough buffers posted in the SRQ? _______________________________________________ openib-general mailing list [email protected] http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
