On Tue, 2015-04-21 at 23:36 +0000, Liran Liss wrote: > Hi Michael, > > The spirit of this patch-set is great, but I think that we need to clarify > some concepts. > Since this will affect the whole patch-set, I am laying out my concerns here > instead. > > A suggestion for the resulting management helpers is given below. > I believe the result would be much more coherent. > --Liran > > In general > ======== > > An ib_dev (or a port of) should be distinguished by 3 qualifiers: > - The link layer: > -- Ethernet (shared by iWARP, USNIC, and ROCE) > -- Infiniband > > - The transport (*) > -- IBTA transport (shared by IB and ROCE) > -- iWARP transport > -- USNIC transport > > (*) Transport means both: > - The L4 wire protocols (e.g., BTH+ headers of IBTA, optionally encapsulated > by UDP in ROCEv2, or the iWARP stack) > - The transport semantics (for example, there are slight semantic differences > between IBTA and iWARP) > > - The node type (**) > -- CA > -- Switch > -- Router > > (**) This has been extended to also encode the transport in the current code. > At least for user-space visible APIs, we might chose to leave this for > backward compatibility, but we can consider cleaning up the kernel code. > > So, I think that our "old-transport" below is just fine. > No need to change it (and you aren't, since it is currently implemented as a > function). > > The "new-transport" does not really exist, but is broken into several > capability checks of the L4 transport, optionally with conditions on the link > type. > I would remove the table below and tell what we really want to achieve: > ==> move technology-specific feature-check logic out of the (multiple!) IB > code components and various ULPs into per-feature helpers. > > > Detailed remarks > ============== > > 1) The introduction of cap_*_*() stuff should have been introduced directly > in patch 02/27. > This back-and-forth between rdma_ib_or_iboe() and cap_* is confusing and > increases the number of patches in the patch-set. > Do this and remove patches 16-24. > > 2)The name rdma_tech_* is lame. > rdma_transport_*(), adhering to the above (*) remark, is much better. > For example, both IB and ROCE *do* use the same transport.
I especially want to second this. I haven't really been happy with the
rdma_tech_* names at all.
--
Doug Ledford <dledf...@redhat.com>
GPG KeyID: 0E572FDD
signature.asc
Description: This is a digitally signed message part
