On Mon, 2015-05-04 at 16:40 +0000, Hefty, Sean wrote:
> > > +/* Protocol                             0xFFFF000000000000 */
> > > +#define RDMA_CORE_CAP_PROT_IB		0x0001000000000000ULL
> > > +#define RDMA_CORE_CAP_PROT_IBOE		0x0002000000000000ULL
> > > +#define RDMA_CORE_CAP_PROT_IWARP	0x0004000000000000ULL
> > > +#define RDMA_CORE_CAP_PROT_USNIC_UDP	0x0008000000000000ULL
> > 
> > In accordance with what we've been talking about, drop IBOE for ROCE.
> > 
> > Drop the UDP off of USNIC, then define a bit for CAP_PROT_UDP_ENCAP.
> > USNIC will be just USNIC, USNIC_UDP will be USNIC | UDP_ENCAP, ROCE v1
> > will be ROCE, and ROCEv2 will be ROCE | UDP_ENCAP.
> 
> USNIC_UDP is just UDP.  I don't understand why we would want 'USNIC | UDP_ENCAP', or what UDP_ENCAP is intended to convey.  Nothing is being encapsulated.

I thought USNIC_UDP had an embedded USNIC protocol header inside the UDP
header.  That would make it a UDP_ENCAP protocol.

> RoCEv2 is IB transport over UDP.

Right, ROCE (or IB, whichever you prefer) encapsulated in UDP.

> I'm not sure what the protocol field is intended to imply.

There is still information in those bits that we can't get elsewhere.
For instance, even though this patch replaces the CAP_* stuff with bits,
if you took away the CAP_PROT_* entries, then there would be no entry to
identify USNIC at all.

Right now, you could infer iWARP from CAP_IW_CM.
You could infer InfiniBand from any of the CAP_IB_* (but later will need
a way to differentiate between IB and OPA)
You could infer ROCE from CAP_ETH_AH (but later will need a way to
differentiate between ROCE and ROCEv2)
The only way to differentiate USNIC at the moment, is that the CAPS
would be all 0.  That's not the sort of positive identification I would
prefer.

So you *could* reduce this to just one bit for USNIC.

And if you then add a UDP_ENCAP bit, then that single bit can do double
duty in telling apart USNIC and USNIC_UDP and ROCE and ROCEv2.

>   If we want
> to expose the link, network, transport, and RDMA protocols in use,
> shouldn't these be separate fields or bits?  And even then, I'm not
> sure what use this has for the ULPs.  iWarp does not require Ethernet
> or TCP.  RoCEv2 would work fine over any link.


>   And the core layer
> should not assume that a device is limited to supporting only one
> protocol, especially at the network and transport levels.

Given that this is a per port thing, there is no assumption about a
device only supporting a single protocol.

>   I vote for
> deprecating the protocol goofiness.

-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
              GPG KeyID: 0E572FDD