[openib-general] Re: [PATCH] [RFC] group devices by type

[Michael S. Tsirkin]

Quoting r. Sean Hefty <sean.hefty at intel.com>:
> Subject: [PATCH] [RFC] group devices by type
> 
> I'd like to get some feedback about adding the ability to group devices
> by some higher level type.  This would permit identifying all devices
> that are of type "Infiniband" from devices of other RDMA transports.
> 
> I've included the patch to ib_verbs.h to do this, along with changes to
> mad.c to show how it would be used.  If this is okay, then similar changes
> would be needed by a dozen or so other files, which I would do before
> submitting a final patch.
> 
> Signed-off-by: Sean Hefty <sean.hefty at intel.com>
> 

Wouldnt a simple helper function be sufficient?
Something like:

int rdma_is_ib_device(enum ib_node_type t)
{
	switch (t) {
	case IB_NODE_CA:
	case IB_NODE_SWITCH:
	case IB_NODE_ROUTER:
		return 1;
	}
	return 0;
}

> ----
> 
> Index: include/rdma/ib_verbs.h
> ===================================================================
> --- include/rdma/ib_verbs.h	(revision 5098)
> +++ include/rdma/ib_verbs.h	(working copy)
> @@ -57,7 +57,8 @@ union ib_gid {
>  };
>  
>  enum ib_node_type {

Flags as enums, hmm.

> -	IB_NODE_CA 	= 1,
> +	IB_NODE_IB = 0x10,		/* mask for all IB node types */
> +	IB_NODE_CA,
>  	IB_NODE_SWITCH,
>  	IB_NODE_ROUTER
>  };

I think this changes the ABI, so its somewhat problematic.
A way to do the same without breaking ABI below.

> Index: core/mad.c
> ===================================================================
> --- core/mad.c	(revision 5098)
> +++ core/mad.c	(working copy)
> @@ -2661,7 +2661,9 @@ static void ib_mad_init_device(struct ib
>  {
>  	int start, end, i;
>  
> -	if (device->node_type == IB_NODE_SWITCH) {
> +	if ((device->node_type & IB_NODE_IB) != IB_NODE_IB)

How about we have

IB_NODE_CA 	= 1,
IB_NODE_SWITCH,
IB_NODE_ROUTER
IB_NODE_MAX

and then you can

if (device->node_type >= IB_NODE_MAX)
	return;

> +		return;
> +	else if (device->node_type == IB_NODE_SWITCH) {
>  		start = 0;
>  		end   = 0;
>  	} else {

-- 
Michael S. Tsirkin
Staff Engineer, Mellanox Technologies