Re: [PATCH v2 01/17] IB/Verbs: Implement new callback query_transport() for each HW

[Jason Gunthorpe <jgunthorpe@obsidianresearch.com>]

On Fri, Apr 10, 2015 at 02:16:11AM -0400, ira.weiny wrote:

> >         IB ROCEE OPA
> > SMI     Y  N     Y    (though the OPA smi looked a bit different)
> 
> Yes OPA is different but it is based on the class version of the individual
> MADs not any particular device/port support.

> > OPA SMP N  N     Y
> 
> How is this different from the SMI?

Any code that generates SMPs and SMIs is going to need to know what
format to generate them in. It seems we have a sort of weird world
where IB SMPs are supported on OPA but not the IB SMI.

Not sure any users exist though..

> > Maybe the above set would make a lot more sense as:
> >   cap_ib_qp0
> >   cap_ib_qp1
> >   cap_opa_qp0
> 
> I disagree.
> 
> All we need right now is is cap_qp0.  All devices currently support QP1.

I didn't list iWarp in the table because everything is no, but it
doesn't support QP1.

> > ib_sa means the IBA SA protocol is supported (Y Y Y)
> 
> I think this should be (Y N Y)
> 
> IBoE has no SA.  The IBoE code "fabricates" a Path Record it does not need to
> interact with the SA.

I was wondering why there are so many checks in the SA code, I know
RoCEE doesn't use it, but why are there there?
 
> > ib_mcast true if the IBA SA protocol is used for multicast GIDs (Y N
> > Y)
> 
> Given the above why can't we just have the "ib_sa" flag?

Maybe I got it wrong, but yes, if it really means 'IBA SA protocol for
multicast then it can just be cap_sa.

But there is also the idea that some devices can't do multicast at all
(iWarp), we must care about that at some point?

> However, I think checking the link layer is more appropriate here.
> It does not make sense to do IP over IB over Eth.  Even though the
> IBoE can do the "IB" protocol.

Yes, it is ugly.

I think if we look closely we'll find that IPoIB today has a hard
requirement on cap_sa being true, so lets use that?

In fact any ULP that unconditionally uses the SA can use that.

> > I actually really prefer cap_mandatory_grh - that is what is going on
> > here. ie based on that name (as a reviewer) I'd expect to see the mad
> > layer check that the mandatory GRH is always present, or blow up.
> 
> While GRH mandatory (for the GMP) is what this is.  The function
> ib_init_ah_from_path generically is really handling an "IBoE address" to send
> to and therefore we need to force the GRH in the AH.

This make sense to me.

It appears we have at least rocee, rocee v2 (udp?), tcp, ib and opa
address and AH formats? opa would support ib addresses too I guess.

A 
 bool rdma_port_addr_is_XXX()

along with a

  enum AddrType rdma_port_addr_type()

Might be the thing? The latter should only be used with switch()

Jason