ib_types.h moving [was: Re: [ofa-general] [RFC] 3/5: IB ACM: libibacm]

ib_types.h moving [was: Re: [ofa-general] [RFC] 3/5: IB ACM: libibacm]

[Sasha Khapyorsky]

On 13:20 Thu 17 Sep     , Ira Weiny wrote:
> 
> Sasha, would you be willing to accept such a patch?  First move ib_types.h to umad and then move the long inline functions into the lib and separate out the remaining header.
> 
> Or would you prefer a new library?  I think there is enough code there but I leave it up to you.

Basically cleaning ib_types.h issue (which was raised repeatedly in the
past too) and making some order here with libibmad duplications would be
a nice thing.

However I still not understand clearly yet how things should be
organized properly (assumig all histories, ibutils dependencies, etc.).
And sure we can try to find an optimal model, so let's discuss:

libibumad is an option. However today this library only provides a
layer to user_mad kernel API and actually is transparent to MAD's
structure. Maybe complicating this with adding ib_types.h and some MAD
fields access helpers is not a big deal, but sort of disadvantage
anyway.

To place this stuff in separate library/package is another possibility,
but perspective of adding new package doesn't make me happy.

In theory ib_types.h would be also merged with libibmad. However for
me the current libibmad seems to be too much heavy for not using it for
stuff other than infiniband-diags.

Another options?

Now I likely would agree with Ira that moving ib_types.h to libibumad
is a least painful option. Do we have a better ideas?

Sasha
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: ib_types.h moving [was: Re: [ofa-general] [RFC] 3/5: IB ACM: libibacm]

[Sean Hefty]

>Now I likely would agree with Ira that moving ib_types.h to libibumad
>is a least painful option. Do we have a better ideas?

Just a random thought, but what about longer term adding a second set of
interfaces to libibumad?  Basically, something more like the kernel ib_sa.  I
don't know that we need a new library just to expand the interface.

For ib_types.h, I'd rather see it broken up into separate header files, at least
some of which get distributed with libibumad.

- Sean

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: ib_types.h moving [was: Re: [ofa-general] [RFC] 3/5: IB ACM: libibacm]

[Ira Weiny]

On Fri, 25 Sep 2009 16:09:08 +0300
Sasha Khapyorsky <sashak-smomgflXvOZWk0Htik3J/w@public.gmane.org> wrote:

> On 13:20 Thu 17 Sep     , Ira Weiny wrote:
> > 
> > Sasha, would you be willing to accept such a patch?  First move ib_types.h to umad and then move the long inline functions into the lib and separate out the remaining header.
> > 
> > Or would you prefer a new library?  I think there is enough code there but I leave it up to you.
> 
> Basically cleaning ib_types.h issue (which was raised repeatedly in the
> past too) and making some order here with libibmad duplications would be
> a nice thing.
> 
> However I still not understand clearly yet how things should be
> organized properly (assumig all histories, ibutils dependencies, etc.).
> And sure we can try to find an optimal model, so let's discuss:
> 
> libibumad is an option. However today this library only provides a
> layer to user_mad kernel API and actually is transparent to MAD's
> structure. Maybe complicating this with adding ib_types.h and some MAD
> fields access helpers is not a big deal, but sort of disadvantage
> anyway.

I think I agree.  I have been looking over the headers and functionality of
the librarys and libibumad is really just an interface to the kernel.  It is
just a data passing library.  libibmad is the library which can marshal
packets (mad packets anyway).  Much of the functionality of ib_types.h is used
in (de|en)codeing packets (mad and others).  For example, there is
functionality for other things such as ib_gid_is_multicast which is used for
non-mad packets.  Where does this functionality go?  I think I agree with Sean
that this header should be broken up but I fear there might be a number of
items left homeless...  :-/

> 
> To place this stuff in separate library/package is another possibility,
> but perspective of adding new package doesn't make me happy.

I think we need to look at libibverbs as well...

from ib_types.h

#define IB_PATH_RECORD_RATE_2_5_GBS		2
#define IB_PATH_RECORD_RATE_5_GBS		5
...
#define IB_MTU_LEN_256							1
#define IB_MTU_LEN_512							2
...
#define IB_LINK_DOWN      1
#define IB_LINK_INIT	  2


>From verbs.h

	IBV_RATE_2_5_GBPS = 2,
	IBV_RATE_5_GBPS   = 5,
...
	IBV_MTU_256  = 1,
	IBV_MTU_512  = 2,
...
	IBV_PORT_DOWN		= 1,
	IBV_PORT_INIT		= 2,


Furthermore, we have 3 functions to decode node type (and their associated
#def/enums).

libibmad:
mad_dump_node_type

ib_types.h:
ib_get_node_type_str

libibverbs:
ibv_node_type_str

I will admit mad_dump_node_type is somewhat special...  But still, why all
this reinvention?

> 
> In theory ib_types.h would be also merged with libibmad. However for
> me the current libibmad seems to be too much heavy for not using it for
> stuff other than infiniband-diags.

I am not quite sure what you mean here.  Do you mean libibmad is already too
complicated ("too much heavy")?  And/Or that libibmad is not appropriate for
users other than infiniband-diags?

> 
> Another options?
> 
> Now I likely would agree with Ira that moving ib_types.h to libibumad
> is a least painful option. Do we have a better ideas?

So far I can only think of 2 "correct" options.

   1) Make ib_types.h into a new library and have libibverbs, libibmad,
      opensm, and MPI's use the constants and helper functions there.  This of
      course would force some dependencies.

   2) split up and spread ib_types.h around to appropriate places.  This will
      likely include libibmad and libibverbs (others?) I don't think libibumad
      is appropriate really.  At first, I suggested this mainly out of
      convenience because WinOF uses it.  However, I don't think it is
      architecturally correct.  libibumad looks to have a sound interface and
      should not be messed with.

Ira

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: ib_types.h moving [was: Re: [ofa-general] [RFC] 3/5: IB ACM: libibacm]

[Jason Gunthorpe]

On Wed, Sep 30, 2009 at 06:31:26PM -0700, Ira Weiny wrote:

> > Now I likely would agree with Ira that moving ib_types.h to libibumad
> > is a least painful option. Do we have a better ideas?
> 
> So far I can only think of 2 "correct" options.
> 
>    1) Make ib_types.h into a new library and have libibverbs, libibmad,
>       opensm, and MPI's use the constants and helper functions there.  This of
>       course would force some dependencies.

You are going the wrong way.. libibverbs is the 'fundamental' lowest
level library. Everything should build on it, not re-invent its stuff.

libibumad should not have device discovery and handling functions!
It should use ibv_device and ibv_context identifiers from libibverbs.

It should not duplicate constants and functions from libibverbs.

>    2) split up and spread ib_types.h around to appropriate places.  This will
>       likely include libibmad and libibverbs (others?) I don't think libibumad
>       is appropriate really.  At first, I suggested this mainly out of
>       convenience because WinOF uses it.  However, I don't think it is
>       architecturally correct.  libibumad looks to have a sound interface and
>       should not be messed with.

It has a horrid interface, it should be destoyed :P

It needs exactly 4 functions:
1) open QP0/QP1 special FD based on ibv_device/ibv_context
2) read/write MADs from the special FD into user buffers. Maybe some
   higher level helper functions (ie send/recv with retry, timeout, etc)
3) Do some basic parsing and whatever of the user buffers (if
   necessary, probably not anymore)
4) Do any special control functions on the QP0/QP1/port
   (ie set control bits, etc, etc)

libibmad should do all the hard core mad processing, structure
definitions, parsing helpers, redirection and other special function
logic, etc. It should be able to ride either on the QP0/QP1 special FD
or directly on a UD QP from libibverbs.

Two libraries are probably not needed, one library containing both
layers (with architectural purity), or putting the umad access layer
in libibverbs would be best. We have too many libraries.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: ib_types.h moving

[Sean Hefty]

>So far I can only think of 2 "correct" options.
>
>   1) Make ib_types.h into a new library and have libibverbs, libibmad,
>      opensm, and MPI's use the constants and helper functions there.  This of
>      course would force some dependencies.
>
>   2) split up and spread ib_types.h around to appropriate places.  This will
>      likely include libibmad and libibverbs (others?) I don't think libibumad
>      is appropriate really.  At first, I suggested this mainly out of
>      convenience because WinOF uses it.  However, I don't think it is
>      architecturally correct.  libibumad looks to have a sound interface and
>      should not be messed with.

The only issue that was pointed out was that the IB ACM duplicated some network
structure definitions (the SA MAD format, McMemberRecord, PathRecord).  It seems
perfectly natural to me to have the library that allows you to send and receive
MADs to provide these definitions, especially considering how fundamental these
specific attributes are to using IB.  Even though umad_send and umad_recv are
both defined to take a void * input buffer, they expect a specific format to
that buffer.

libibverbs defines host structure definitions for McMemberRecord and PathRecord,
but doesn't use them.  This allows the rdma_cm and ib_cm to remain independent,
but use the same definition.

IB ACM is coded to libibumad directly, mainly because I didn't see any benefit
to coding to a higher level MAD interface that was more complex to use.  Trying
to keep the defines in a higher level library, which is what we have now, either
results in duplicating them or forcing a dependency where there wouldn't be any.

And to be clear, I'm only referring to definitions in ib_types.h (and some in
mad.h), and not any actual functionality.

- Sean

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: ib_types.h moving [was: Re: [ofa-general] [RFC] 3/5: IB ACM: libibacm]

[Ira Weiny]

On Wed, 30 Sep 2009 20:57:52 -0600
Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> wrote:

> On Wed, Sep 30, 2009 at 06:31:26PM -0700, Ira Weiny wrote:
> 
> > > Now I likely would agree with Ira that moving ib_types.h to libibumad
> > > is a least painful option. Do we have a better ideas?
> > 
> > So far I can only think of 2 "correct" options.
> > 
> >    1) Make ib_types.h into a new library and have libibverbs, libibmad,
> >       opensm, and MPI's use the constants and helper functions there.  This of
> >       course would force some dependencies.
> 
> You are going the wrong way.. libibverbs is the 'fundamental' lowest
> level library. Everything should build on it, not re-invent its stuff.

I agree, however, I think others do not.  In particular is Roland ready to
accept a significant portion of ib_types.h?  I don't have any problem
requiring libibverbs for anything IB related.  OTOH, I am not so against
having many libraries as long as they are well defined and well known.

If the only solution is to have one massive libib library, so be it!  I don't
have a problem with that.  But I think the maintainer of this lib is going to
have a hard job.  And perhaps some (switch vendors?) might want at least a
little more break down of functionality to pick and choose what is required?

> 
> libibumad should not have device discovery and handling functions!
> It should use ibv_device and ibv_context identifiers from libibverbs.

I also agree, however, ib_types does not have any device discover functions.
I was only speaking of where to put the ib_types functionality.  Not about
other problems which may exist.  ;-)

> 
> It should not duplicate constants and functions from libibverbs.

Absolutely!  Sasha are you ready to make OpenSM depend on libibverbs?

> 
> >    2) split up and spread ib_types.h around to appropriate places.  This will
> >       likely include libibmad and libibverbs (others?) I don't think libibumad
> >       is appropriate really.  At first, I suggested this mainly out of
> >       convenience because WinOF uses it.  However, I don't think it is
> >       architecturally correct.  libibumad looks to have a sound interface and
> >       should not be messed with.
> 
> It has a horrid interface, it should be destoyed :P
> 
> It needs exactly 4 functions:
> 1) open QP0/QP1 special FD based on ibv_device/ibv_context
> 2) read/write MADs from the special FD into user buffers. Maybe some
>    higher level helper functions (ie send/recv with retry, timeout, etc)
> 3) Do some basic parsing and whatever of the user buffers (if
>    necessary, probably not anymore)
> 4) Do any special control functions on the QP0/QP1/port
>    (ie set control bits, etc, etc)

Ok, ok.  Once again I was only speaking of where to put the ib_types stuff.  I
think you would agree that adding ib_types stuff to libibumad is the wrong
direction, right?

>
> 
> libibmad should do all the hard core mad processing, structure
> definitions, parsing helpers, redirection and other special function
> logic, etc. It should be able to ride either on the QP0/QP1 special FD
> or directly on a UD QP from libibverbs.

Yes, yes, and yes!

> 
> Two libraries are probably not needed, one library containing both
> layers (with architectural purity), or putting the umad access layer
> in libibverbs would be best. We have too many libraries.

Ok, as I said one massive libib library is fine with me.  Are Roland and Sasha
ready to accept this?

One final thought.  Does WinOF use libibverbs as well?  Would moving ib_types
functionality in to libibverbs be a problem for them?

Ira


-- 
Ira Weiny
Math Programmer/Computer Scientist
Lawrence Livermore National Lab
925-423-8008
weiny2-i2BcT+NCU+M@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: ib_types.h moving [was: Re: [ofa-general] [RFC] 3/5: IB ACM: libibacm]

[Sean Hefty]

>One final thought.  Does WinOF use libibverbs as well?  Would moving ib_types
>functionality in to libibverbs be a problem for them?

Actually libibumad in WinOF uses libibverbs to get the device information.

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: ib_types.h moving [was: Re: [ofa-general] [RFC] 3/5: IB ACM: libibacm]

[Jason Gunthorpe]

On Thu, Oct 01, 2009 at 03:50:07PM -0700, Ira Weiny wrote:
> On Wed, 30 Sep 2009 20:57:52 -0600
> Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> wrote:
> 
> > On Wed, Sep 30, 2009 at 06:31:26PM -0700, Ira Weiny wrote:
> > 
> > > > Now I likely would agree with Ira that moving ib_types.h to libibumad
> > > > is a least painful option. Do we have a better ideas?
> > > 
> > > So far I can only think of 2 "correct" options.
> > > 
> > >    1) Make ib_types.h into a new library and have libibverbs, libibmad,
> > >       opensm, and MPI's use the constants and helper functions there.  This of
> > >       course would force some dependencies.
> > 
> > You are going the wrong way.. libibverbs is the 'fundamental' lowest
> > level library. Everything should build on it, not re-invent its stuff.

> I agree, however, I think others do not.  In particular is Roland ready to
> accept a significant portion of ib_types.h?  I don't have any problem
> requiring libibverbs for anything IB related.  OTOH, I am not so against
> having many libraries as long as they are well defined and well known.

Well, I'm not so sure a significant amount of ib_types.h belongs in
ibverbs either, alot of that is just OSM junk, and it needs some
cleanup to be part of a stable ABI library

Lets just browse here..

- RMPP stuff, libibumad, libibumad would also have the low level RMPP
  kernel/user interfacing
- QPN numbers, well known values, (PKEY, QKEY, LIDS, MCAST, SUBNET PERFIX) sounds
  like good ibverbs material if it isn't there since it is usable for some of the
  ibverbs calls
- ib_gid_t (alike, but different) is already in ibverbs, gid
  processing functions belong in ibverbs ib_gid_is_multicast
  ib_mgid_get_scope ib_mgid_set_scope ib_gid_set_default
  ib_gid_get_subnet_prefix  ib_gid_is_link_local ib_gid_is_site_local ib_gid_get_guid
- MTU constants duplicates ibverbs constants
- ib_path_rec_t is part of ibverbs, ib_path_* could be in ibverbs too

- Random IB MAD processing junk, random choosen things like:
  IB_NODE_NUM_PORTS_MAX IB_SUBNET_PATH_HOPS_MAX
  IB_PKEY_MAX_BLOCKS IB_MCAST_BLOCK_ID_MASK_HO IB_PKEY_BASE_MASK  IB_MCLASS_BM
  IB_MCLASS_VENDOR_HIGH_RANGE_MAX ib_class_is_vendor_specific_low
  IB_MAD_METHOD_RESP_MASK IB_MAD_METHOD_SET IB_MAD_ATTR_NOTICE
  IB_MAD_ATTR_P_KEY_TABLE IB_PR_COMPMASK_SERVICEID_MSB
  general mad containers and related
  ib_class_port_info_t,ib_sm_info_t, ib_smp_t ib_port_info_t  ..
  can go in libibmad ib mad decoding

>From about line 9175 to the end I'd say that duplicates ibverbs and
should refer to ibverbs versions, or go away entirely (dead code?)

> If the only solution is to have one massive libib library, so be it!  I don't
> have a problem with that.  But I think the maintainer of this lib is going to
> have a hard job.  And perhaps some (switch vendors?) might want at least a
> little more break down of functionality to pick and choose what is required?

The nice thing about libraries is you don't have to include everything
in them in your embedded applications - there are multiple techniques
for using only the subset of object files that you actually use. What
is important is that the library has sensible .o file selection so
that these schemes work. For instance a library with several
orthogonal API groupings should at least put each grouping in a
seperate .o file.

But ultimately this is all fairly static code, the IBA doesn't change
very much and all the MAD processing goop is nice and constant.

> > libibumad should not have device discovery and handling functions!
> > It should use ibv_device and ibv_context identifiers from libibverbs.
> 
> I also agree, however, ib_types does not have any device discover functions.
> I was only speaking of where to put the ib_types functionality.  Not about
> other problems which may exist.  ;-)

It does have  stuff like ib_ca_attr_t ib_port_attr_t which are part of
a discovery thingy (dead code?) :)

> Ok, ok.  Once again I was only speaking of where to put the ib_types stuff.  I
> think you would agree that adding ib_types stuff to libibumad is the wrong
> direction, right?

Well, I bring this all up because ib_types is a huge file and it needs
to go into alot of different places. It is impossible to really judge
where to put things without a clear vision of what the net result
should look like.

Something like:
 - ibverbs controls the device access, naming and base IB types for
   LID, GID and PKEY addressing
 - libumad provides a thin user layer for the kernel QP0 and QP1 special
   QPs, including access to the kernel RMPP engine.
 - libibmad provides all the structure parsing and message processing
   code
   libibmad provides a sane IB address resolution layer (think getaddrinfo but for IB)
 - libibcm provides interfaces that are bolt together compatible with all
   the above libraries.

Now maybe some of those layers get smushed together, libibcm and
libumad are both excessively small libraries :)

> > Two libraries are probably not needed, one library containing both
> > layers (with architectural purity), or putting the umad access layer
> > in libibverbs would be best. We have too many libraries.
 
> Ok, as I said one massive libib library is fine with me.  Are Roland and Sasha
> ready to accept this?

I think the stuff that would want to be added in libibverbs is pretty
minor, mostly some addressing helpers and additional constants that
are fairly static. There isn't really a long term churn there.

Next, I think strategies to get the libraries to the above kind of
layered state need to be devised. Ie for libumad I'd say crank the
SONAME and start again. It is small enough that is very easy, and the
current API really has nothing worth keeping.

Not sure what approach works for libibmad..

Also like I said, I have some stuff that is a different approach than
ib_types, that is much easier to use (bit fields, vs explicit bit
bashing) if it ports properly to Windows MS VC it might be worth
considering going that route instead.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: ib_types.h moving

[Ira Weiny]

On Thu, 1 Oct 2009 14:47:33 -0700
"Sean Hefty" <sean.hefty-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:

> >So far I can only think of 2 "correct" options.
> >
> >   1) Make ib_types.h into a new library and have libibverbs, libibmad,
> >      opensm, and MPI's use the constants and helper functions there.  This of
> >      course would force some dependencies.
> >
> >   2) split up and spread ib_types.h around to appropriate places.  This will
> >      likely include libibmad and libibverbs (others?) I don't think libibumad
> >      is appropriate really.  At first, I suggested this mainly out of
> >      convenience because WinOF uses it.  However, I don't think it is
> >      architecturally correct.  libibumad looks to have a sound interface and
> >      should not be messed with.
> 
> The only issue that was pointed out was that the IB ACM duplicated some network
> structure definitions (the SA MAD format, McMemberRecord, PathRecord).  It seems
> perfectly natural to me to have the library that allows you to send and receive
> MADs to provide these definitions, especially considering how fundamental these
> specific attributes are to using IB.  Even though umad_send and umad_recv are
> both defined to take a void * input buffer, they expect a specific format to
> that buffer.

Combine libibmad into libibumad?  Or just the core structure definitions?  I
will admit that accepting void * seems wrong when you know it is
supposed to be a mad packet.  In fact both umad_send and umad_recv imediately
cast the void * to struct ib_user_mad *!  Why not specify that in the
parameter type?

> 
> libibverbs defines host structure definitions for McMemberRecord and PathRecord,
> but doesn't use them.  This allows the rdma_cm and ib_cm to remain independent,
> but use the same definition.

:-(  I missed those.  Is there a reason they must remain independent?  (I
assume you mean independent of the mad libs?)

> 
> IB ACM is coded to libibumad directly, mainly because I didn't see any benefit
> to coding to a higher level MAD interface that was more complex to use.  Trying
> to keep the defines in a higher level library, which is what we have now, either
> results in duplicating them or forcing a dependency where there wouldn't be any.

I understand the desire to avoid using libibmad.  Perhaps you are correct that
there are some things which are "core" MAD functionality, structure
definitions for example, which should be at a lower level.  However, I think
it is still appropriate to put some things from ib_types into libibverbs and
to expect people to use those definitions when required.

> 
> And to be clear, I'm only referring to definitions in ib_types.h (and some in
> mad.h), and not any actual functionality.
> 

<sigh> yes this discussion has digressed.  But I don't think we should dive
into breaking up (or moving) ib_types just to move things around.  I think
Sasha is right we need to have a plan for where things _should_ end up.  As I
look for homes for all the items of ib_types I find that it is not just one
place.  :-/

Ira

-- 
Ira Weiny
Math Programmer/Computer Scientist
Lawrence Livermore National Lab
925-423-8008
weiny2-i2BcT+NCU+M@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: ib_types.h moving

[Sean Hefty]

><sigh> yes this discussion has digressed.  But I don't think we should dive
>into breaking up (or moving) ib_types just to move things around.  I think
>Sasha is right we need to have a plan for where things _should_ end up.  As I
>look for homes for all the items of ib_types I find that it is not just one
>place.  :-/

I really don't think it's worth the effort to rework IB libraries and MAD
layers.  Most user space applications need simpler interfaces to use than
communicating directly with the IB SA.  For the few that want or need to, the
existing libraries are sufficient.  We only need a better place for some of the
structure definitions than an opensm header file.  Even the ib-diags shouldn't
depend on opensm.

- Sean

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: ib_types.h moving [was: Re: [ofa-general] [RFC] 3/5: IB ACM: libibacm]

[Ira Weiny]

On Thu, 1 Oct 2009 17:39:15 -0600
Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> wrote:

> On Thu, Oct 01, 2009 at 03:50:07PM -0700, Ira Weiny wrote:
> > On Wed, 30 Sep 2009 20:57:52 -0600
> > Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> wrote:
> > 
> > > On Wed, Sep 30, 2009 at 06:31:26PM -0700, Ira Weiny wrote:
> > > 
> > > > > Now I likely would agree with Ira that moving ib_types.h to libibumad
> > > > > is a least painful option. Do we have a better ideas?
> > > > 
> > > > So far I can only think of 2 "correct" options.
> > > > 
> > > >    1) Make ib_types.h into a new library and have libibverbs, libibmad,
> > > >       opensm, and MPI's use the constants and helper functions there.  This of
> > > >       course would force some dependencies.
> > > 
> > > You are going the wrong way.. libibverbs is the 'fundamental' lowest
> > > level library. Everything should build on it, not re-invent its stuff.
> 
> > I agree, however, I think others do not.  In particular is Roland ready to
> > accept a significant portion of ib_types.h?  I don't have any problem
> > requiring libibverbs for anything IB related.  OTOH, I am not so against
> > having many libraries as long as they are well defined and well known.
> 
> Well, I'm not so sure a significant amount of ib_types.h belongs in
> ibverbs either, alot of that is just OSM junk, and it needs some
> cleanup to be part of a stable ABI library
> 
> Lets just browse here..
> 
> - RMPP stuff, libibumad, libibumad would also have the low level RMPP
>   kernel/user interfacing
> - QPN numbers, well known values, (PKEY, QKEY, LIDS, MCAST, SUBNET PERFIX) sounds
>   like good ibverbs material if it isn't there since it is usable for some of the
>   ibverbs calls
> - ib_gid_t (alike, but different) is already in ibverbs, gid
>   processing functions belong in ibverbs ib_gid_is_multicast
>   ib_mgid_get_scope ib_mgid_set_scope ib_gid_set_default
>   ib_gid_get_subnet_prefix  ib_gid_is_link_local ib_gid_is_site_local ib_gid_get_guid
> - MTU constants duplicates ibverbs constants
> - ib_path_rec_t is part of ibverbs, ib_path_* could be in ibverbs too

Except for the RMPP stuff you list first, this is all moving to (or already
duplicated by) ibverbs.  I have not counted the lines of code but this seems
like more than an insignificant change to ibverbs.  Furthermore, it would mean
significant build changes to OpenSM and the diags.  Mind you, this is not
insurmountable but I think there is some work to be done here.

> 
> - Random IB MAD processing junk, random choosen things like:
>   IB_NODE_NUM_PORTS_MAX IB_SUBNET_PATH_HOPS_MAX
>   IB_PKEY_MAX_BLOCKS IB_MCAST_BLOCK_ID_MASK_HO IB_PKEY_BASE_MASK  IB_MCLASS_BM
>   IB_MCLASS_VENDOR_HIGH_RANGE_MAX ib_class_is_vendor_specific_low
>   IB_MAD_METHOD_RESP_MASK IB_MAD_METHOD_SET IB_MAD_ATTR_NOTICE
>   IB_MAD_ATTR_P_KEY_TABLE IB_PR_COMPMASK_SERVICEID_MSB
>   general mad containers and related
>   ib_class_port_info_t,ib_sm_info_t, ib_smp_t ib_port_info_t  ..
>   can go in libibmad ib mad decoding
> 
> From about line 9175 to the end I'd say that duplicates ibverbs and
> should refer to ibverbs versions, or go away entirely (dead code?)

Yep!  I don't see anyone using it anyway.  Perhaps a first step is to start
removing dead code?

> 
> > If the only solution is to have one massive libib library, so be it!  I don't
> > have a problem with that.  But I think the maintainer of this lib is going to
> > have a hard job.  And perhaps some (switch vendors?) might want at least a
> > little more break down of functionality to pick and choose what is required?
> 
> The nice thing about libraries is you don't have to include everything
> in them in your embedded applications - there are multiple techniques
> for using only the subset of object files that you actually use. What
> is important is that the library has sensible .o file selection so
> that these schemes work. For instance a library with several
> orthogonal API groupings should at least put each grouping in a
> seperate .o file.
> 
> But ultimately this is all fairly static code, the IBA doesn't change
> very much and all the MAD processing goop is nice and constant.

I get all that...  I just did not want to rock the boat too much, lest I get
thrown out...  ;-)  Perhaps I am past that point already?

> 
> > > libibumad should not have device discovery and handling functions!
> > > It should use ibv_device and ibv_context identifiers from libibverbs.
> > 
> > I also agree, however, ib_types does not have any device discover functions.
> > I was only speaking of where to put the ib_types functionality.  Not about
> > other problems which may exist.  ;-)
> 
> It does have  stuff like ib_ca_attr_t ib_port_attr_t which are part of
> a discovery thingy (dead code?) :)

:-/  not dead...  But specific to the vendor layer in opensm.  So leave that
in the OpenSM tree for now, until we can determine if the other vendor layers
are used and by whom.  I see some more work to be done in this area but I am
unsure of the current use of the vendor layer.  Hal often asks the list and
no one replies to him...  :-/  It seems that Windows is on board with using
libibumad and I don't know of any other OpenSM user who does not.
Furthermore, I don't know why OpenSM could not use libib[u]mad definitions
internally even if the vendor layer is suppling wire packets from some other
vendor defined structure.  I don't care if vendors want to define their own
proprietary versions of these definitions and structs.  ;-)  Well as long as
they follow the spec, of course.

> 
> > Ok, ok.  Once again I was only speaking of where to put the ib_types stuff.  I
> > think you would agree that adding ib_types stuff to libibumad is the wrong
> > direction, right?
> 
> Well, I bring this all up because ib_types is a huge file and it needs
> to go into alot of different places. It is impossible to really judge
> where to put things without a clear vision of what the net result
> should look like.

Agreed.

> 
> Something like:
>  - ibverbs controls the device access, naming and base IB types for
>    LID, GID and PKEY addressing
>  - libumad provides a thin user layer for the kernel QP0 and QP1 special
>    QPs, including access to the kernel RMPP engine.
>  - libibmad provides all the structure parsing and message processing
>    code
>    libibmad provides a sane IB address resolution layer (think getaddrinfo but for IB)
>  - libibcm provides interfaces that are bolt together compatible with all
>    the above libraries.
> 
> Now maybe some of those layers get smushed together, libibcm and
> libumad are both excessively small libraries :)

Agreed.  I am more than willing to create patches and work through this.
However, I want to agree on a plan first.[*]  Then we can keep the thrashing of
large and complex patches to a minimum.

> 
> > > Two libraries are probably not needed, one library containing both
> > > layers (with architectural purity), or putting the umad access layer
> > > in libibverbs would be best. We have too many libraries.
>  
> > Ok, as I said one massive libib library is fine with me.  Are Roland and Sasha
> > ready to accept this?
> 
> I think the stuff that would want to be added in libibverbs is pretty
> minor, mostly some addressing helpers and additional constants that
> are fairly static. There isn't really a long term churn there.

I hope not.  And I tend to agree with you.  But I fear things I don't
understand like ibutils.  I don't know what it needs or where it draws what
from.  We might seriously break them.  Maintainers of other projects need to
know this is happening and, at a minimum, understand why.  I think things will
go much easier if they do.

> 
> Next, I think strategies to get the libraries to the above kind of
> layered state need to be devised. Ie for libumad I'd say crank the
> SONAME and start again. It is small enough that is very easy, and the
> current API really has nothing worth keeping.
> 
> Not sure what approach works for libibmad..

[*]How about this for a plan:

   1) remove dead and duplicated code from ib_types.h
   2) combine libibmad and libibumad into new libibumad2
      2a) leave dead/outdated code out of libibumad2
   3) move verbs ib_types functionality into libibverbs
      3a) adjust OpenSM and vendor layers as appropriate
   4) Move MAD ib_types functionality into libibumad2
      4a) convert upper vendor layer and OpenSM to use new MAD defines in
          libibumad2
   5) remove ib_types.h

I am probably missing something.

> 
> Also like I said, I have some stuff that is a different approach than
> ib_types, that is much easier to use (bit fields, vs explicit bit
> bashing) if it ports properly to Windows MS VC it might be worth
> considering going that route instead.

That sounds interesting.  I just don't want to see further duplication.  It's
just too confusing.  I know that my brain is the smallest of the bunch here.
So maybe that is why I am suffering the most with the current complexity...
:-/  ???

Ira

-- 
Ira Weiny
Math Programmer/Computer Scientist
Lawrence Livermore National Lab
925-423-8008
weiny2-i2BcT+NCU+M@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: ib_types.h moving [was: Re: [ofa-general] [RFC] 3/5: IB ACM: libibacm]

[Hal Rosenstock]

On Fri, Oct 2, 2009 at 1:45 PM, Ira Weiny <weiny2-i2BcT+NCU+M@public.gmane.org> wrote:
> On Thu, 1 Oct 2009 17:39:15 -0600
> Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> wrote:
>
>> On Thu, Oct 01, 2009 at 03:50:07PM -0700, Ira Weiny wrote:
>> > On Wed, 30 Sep 2009 20:57:52 -0600
>> > Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> wrote:
>> >
>> > > On Wed, Sep 30, 2009 at 06:31:26PM -0700, Ira Weiny wrote:
>> > >
>> > > > > Now I likely would agree with Ira that moving ib_types.h to libibumad
>> > > > > is a least painful option. Do we have a better ideas?
>> > > >
>> > > > So far I can only think of 2 "correct" options.
>> > > >
>> > > >    1) Make ib_types.h into a new library and have libibverbs, libibmad,
>> > > >       opensm, and MPI's use the constants and helper functions there.  This of
>> > > >       course would force some dependencies.
>> > >
>> > > You are going the wrong way.. libibverbs is the 'fundamental' lowest
>> > > level library. Everything should build on it, not re-invent its stuff.
>>
>> > I agree, however, I think others do not.  In particular is Roland ready to
>> > accept a significant portion of ib_types.h?  I don't have any problem
>> > requiring libibverbs for anything IB related.  OTOH, I am not so against
>> > having many libraries as long as they are well defined and well known.
>>
>> Well, I'm not so sure a significant amount of ib_types.h belongs in
>> ibverbs either, alot of that is just OSM junk, and it needs some
>> cleanup to be part of a stable ABI library
>>
>> Lets just browse here..
>>
>> - RMPP stuff, libibumad, libibumad would also have the low level RMPP
>>   kernel/user interfacing
>> - QPN numbers, well known values, (PKEY, QKEY, LIDS, MCAST, SUBNET PERFIX) sounds
>>   like good ibverbs material if it isn't there since it is usable for some of the
>>   ibverbs calls
>> - ib_gid_t (alike, but different) is already in ibverbs, gid
>>   processing functions belong in ibverbs ib_gid_is_multicast
>>   ib_mgid_get_scope ib_mgid_set_scope ib_gid_set_default
>>   ib_gid_get_subnet_prefix  ib_gid_is_link_local ib_gid_is_site_local ib_gid_get_guid
>> - MTU constants duplicates ibverbs constants
>> - ib_path_rec_t is part of ibverbs, ib_path_* could be in ibverbs too
>
> Except for the RMPP stuff you list first, this is all moving to (or already
> duplicated by) ibverbs.  I have not counted the lines of code but this seems
> like more than an insignificant change to ibverbs.  Furthermore, it would mean
> significant build changes to OpenSM and the diags.  Mind you, this is not
> insurmountable but I think there is some work to be done here.
>
>>
>> - Random IB MAD processing junk, random choosen things like:
>>   IB_NODE_NUM_PORTS_MAX IB_SUBNET_PATH_HOPS_MAX
>>   IB_PKEY_MAX_BLOCKS IB_MCAST_BLOCK_ID_MASK_HO IB_PKEY_BASE_MASK  IB_MCLASS_BM
>>   IB_MCLASS_VENDOR_HIGH_RANGE_MAX ib_class_is_vendor_specific_low
>>   IB_MAD_METHOD_RESP_MASK IB_MAD_METHOD_SET IB_MAD_ATTR_NOTICE
>>   IB_MAD_ATTR_P_KEY_TABLE IB_PR_COMPMASK_SERVICEID_MSB
>>   general mad containers and related
>>   ib_class_port_info_t,ib_sm_info_t, ib_smp_t ib_port_info_t  ..
>>   can go in libibmad ib mad decoding
>>
>> From about line 9175 to the end I'd say that duplicates ibverbs and
>> should refer to ibverbs versions, or go away entirely (dead code?)
>
> Yep!  I don't see anyone using it anyway.

I think this is used in Windows and may be used by out of tree users.
I know this can move but it is a consideration (at least IMO).

> Perhaps a first step is to start
> removing dead code?
>
>>
>> > If the only solution is to have one massive libib library, so be it!  I don't
>> > have a problem with that.  But I think the maintainer of this lib is going to
>> > have a hard job.  And perhaps some (switch vendors?) might want at least a
>> > little more break down of functionality to pick and choose what is required?
>>
>> The nice thing about libraries is you don't have to include everything
>> in them in your embedded applications - there are multiple techniques
>> for using only the subset of object files that you actually use. What
>> is important is that the library has sensible .o file selection so
>> that these schemes work. For instance a library with several
>> orthogonal API groupings should at least put each grouping in a
>> seperate .o file.
>>
>> But ultimately this is all fairly static code, the IBA doesn't change
>> very much and all the MAD processing goop is nice and constant.
>
> I get all that...  I just did not want to rock the boat too much, lest I get
> thrown out...  ;-)  Perhaps I am past that point already?
>
>>
>> > > libibumad should not have device discovery and handling functions!
>> > > It should use ibv_device and ibv_context identifiers from libibverbs.
>> >
>> > I also agree, however, ib_types does not have any device discover functions.
>> > I was only speaking of where to put the ib_types functionality.  Not about
>> > other problems which may exist.  ;-)
>>
>> It does have  stuff like ib_ca_attr_t ib_port_attr_t which are part of
>> a discovery thingy (dead code?) :)
>
> :-/  not dead...  But specific to the vendor layer in opensm.

For at least umad vendor layer, the discovery code could be changed to
use verbs rather than the current umad calls for this. It sounded like
Sean already did this for Windows. I wonder whether those changes can
readily be transferred to Linux.

> So leave that
> in the OpenSM tree for now, until we can determine if the other vendor layers
> are used and by whom.  I see some more work to be done in this area but I am
> unsure of the current use of the vendor layer.  Hal often asks the list and
> no one replies to him...  :-/  It seems that Windows is on board with using
> libibumad and I don't know of any other OpenSM user who does not.
> Furthermore, I don't know why OpenSM could not use libib[u]mad definitions
> internally even if the vendor layer is suppling wire packets from some other
> vendor defined structure.  I don't care if vendors want to define their own
> proprietary versions of these definitions and structs.  ;-)  Well as long as
> they follow the spec, of course.
>
>>
>> > Ok, ok.  Once again I was only speaking of where to put the ib_types stuff.  I
>> > think you would agree that adding ib_types stuff to libibumad is the wrong
>> > direction, right?
>>
>> Well, I bring this all up because ib_types is a huge file and it needs
>> to go into alot of different places. It is impossible to really judge
>> where to put things without a clear vision of what the net result
>> should look like.
>
> Agreed.
>
>>
>> Something like:
>>  - ibverbs controls the device access, naming and base IB types for
>>    LID, GID and PKEY addressing
>>  - libumad provides a thin user layer for the kernel QP0 and QP1 special
>>    QPs, including access to the kernel RMPP engine.
>>  - libibmad provides all the structure parsing and message processing
>>    code
>>    libibmad provides a sane IB address resolution layer (think getaddrinfo but for IB)
>>  - libibcm provides interfaces that are bolt together compatible with all
>>    the above libraries.
>>
>> Now maybe some of those layers get smushed together, libibcm and
>> libumad are both excessively small libraries :)
>
> Agreed.  I am more than willing to create patches and work through this.
> However, I want to agree on a plan first.[*]  Then we can keep the thrashing of
> large and complex patches to a minimum.
>
>>
>> > > Two libraries are probably not needed, one library containing both
>> > > layers (with architectural purity), or putting the umad access layer
>> > > in libibverbs would be best. We have too many libraries.
>>
>> > Ok, as I said one massive libib library is fine with me.  Are Roland and Sasha
>> > ready to accept this?
>>
>> I think the stuff that would want to be added in libibverbs is pretty
>> minor, mostly some addressing helpers and additional constants that
>> are fairly static. There isn't really a long term churn there.
>
> I hope not.  And I tend to agree with you.  But I fear things I don't
> understand like ibutils.  I don't know what it needs or where it draws what
> from.  We might seriously break them.  Maintainers of other projects need to
> know this is happening and, at a minimum, understand why.  I think things will
> go much easier if they do.
>
>>
>> Next, I think strategies to get the libraries to the above kind of
>> layered state need to be devised. Ie for libumad I'd say crank the
>> SONAME and start again. It is small enough that is very easy, and the
>> current API really has nothing worth keeping.
>>
>> Not sure what approach works for libibmad..
>
> [*]How about this for a plan:
>
>   1) remove dead and duplicated code from ib_types.h
>   2) combine libibmad and libibumad into new libibumad2
>      2a) leave dead/outdated code out of libibumad2
>   3) move verbs ib_types functionality into libibverbs
>      3a) adjust OpenSM and vendor layers as appropriate
>   4) Move MAD ib_types functionality into libibumad2
>      4a) convert upper vendor layer and OpenSM to use new MAD defines in
>          libibumad2
>   5) remove ib_types.h
>
> I am probably missing something.

What about ibsim for one ?

-- Hal

>>
>> Also like I said, I have some stuff that is a different approach than
>> ib_types, that is much easier to use (bit fields, vs explicit bit
>> bashing) if it ports properly to Windows MS VC it might be worth
>> considering going that route instead.
>
> That sounds interesting.  I just don't want to see further duplication.  It's
> just too confusing.  I know that my brain is the smallest of the bunch here.
> So maybe that is why I am suffering the most with the current complexity...
> :-/  ???
>
> Ira
>
> --
> Ira Weiny
> Math Programmer/Computer Scientist
> Lawrence Livermore National Lab
> 925-423-8008
> weiny2-i2BcT+NCU+M@public.gmane.org
>
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html