Re: [RFC PATCH 06/11] IB/Verbs: Use management helper has_sa() and cap_sa(), for sa-check

[Doug Ledford]

On Mon, 2015-03-30 at 18:42 +0200, Michael Wang wrote:
> On 03/30/2015 06:16 PM, Doug Ledford wrote:
> > On Fri, 2015-03-27 at 16:46 +0100, Michael Wang wrote:
> >> Introduce helper has_sa() and cap_sa() to help us check if an IB device
> >> or it's port support Subnet Administrator.
> > There's no functional reason to have both rdma_transport_is_ib and
> > rdma_port_ll_is_ib, just use one.  Then there is also no reason for both
> > has_sa and cap_sa.  Just use one.
> The has_sa() will be eliminated :-)
> 
> rdma_transport_is_ib and rdma_port_ll_is_ib is actually just rough helper
> to save some code, we can get rid of them when we no longer need them, but
> currently device driver still using them a lot, I'm not sure if the new
> mechanism could take cover all these cases...

Sure it would.  This is what I had suggested (well, close to this, I
rearranged the order this time around):

enum rdma_transport {
	RDMA_TRANSPORT_IB =      0x01,
	RDMA_TRANSPORT_OPA =     0x02,
	RDMA_TRANSPORT_IWARP =   0x04,
	RDMA_TRANSPORT_ROCE_V1 = 0x08,
	RDMA_TRANSPORT_ROCE_V2 = 0x10,
};

struct ib_port {
	...
	enum rdma_transport;
	...
};

static inline bool rdma_transport_is_ib(struct ib_port *port)
{
	return port->transport & (RDMA_TRANSPORT_IB | RDMA_TRANSPORT_OPA);
}

static inline bool rdma_transport_is_opa(struct ib_port *port)
{
	return port->transport & RDMA_TRANSPORT_OPA;
}

static inline bool rdma_transport_is_iwarp(struct ib_port *port)
{
	return port->transport & RDMA_TRANSPORT_IWARP;
}

static inline bool rdma_transport_is_roce(struct ib_port *port)
{
	return port->transport & (RDMA_TRANSPORT_ROCE_V1 | RDMA_TRANSPORT_ROCE_V2);
}

static inline bool rdma_ib_mgmt(struct ib_port *port)
{
	return port->transport & (RDMA_TRANSPORT_IB | RDMA_TRANSPORT_OPA);
}

static inline bool rdma_opa_mgmt(struct ib_port *port)
{
	return port->transport & RDMA_TRANSPORT_OPA;
}


If we use something like this, then the above is all you need.  Then
every place in the code that checks for something like has_sa or cap_sa
can be replaced with rdma_ib_mgmt.  When Ira updates his patches for
this, he can check for rdma_opa_mgmt to enable jumbo MAD packets and
whatever else he needs.  Every place that does transport == IB and ll ==
Ethernet can become rdma_transport_is_roce.  Every place that does
transport == IB and ll == INFINIBAND becomes rdma_transport_is_ib.  The
code in multicast.c just needs to check rdma_ib_mgmt() (which happens to
make perfect sense anyway as the code in multicast.c that is checking
that we are on an IB interface is doing so because IB requires extra
management of the multicast group joins/leaves).  But, like I said, this
is an all or nothing change, it isn't something we can ease into.

-- 
Doug Ledford <dledford@xxxxxxxxxx>
              GPG KeyID: 0E572FDD

Attachment: signature.asc
Description: This is a digitally signed message part