Upcoming libibverbs release

Upcoming libibverbs release

[Doug Ledford]

[-- Attachment #1: Type: text/plain, Size: 3746 bytes --]

I'm preparing to make an official release of libibverbs soonish
(thinking by the end of next week).  Even though I took the timestamp v5
patchset, I know there were still comments from Jason on the v4 set that
weren't addressed in v5.  Those can be addressed (and should be) with
incremental patches prior to the official release.  Yishai, can we
please prioritize getting that done?

Here's the git changelog from the official 1.2.0 release to the current
HEAD:

7f272c00424b Update .gitignore
20e2daec4578 Fail compiles if no platform specific memory barriers exist
0de26fb8b62d libibverbs: add ARM64 memory barrier macros
d0e4e455b96e Add basic man page entry for ibv_xsrq_pingpong
8f59a5c28654 Merge remote-tracking branch 'yishaih/fcs'
2928832d0649 Add timestamp support in rc_pingpong
f4e6a85248fa Add a verb that queries real time values from the HCA
169a6efad9e5 Create a single threaded CQ
c2e36f6797f4 Add completion timestamp to poll_cq
1c8a921d0227 Add timestamp_mask and hca_core_clock to ibv_query_device_ex
10f97b68181a Add member functions to poll an extended CQ
4ace2118493b Add support for extended creating CQ verb
5852b93d3bcd Add SCATTER_FCS QP create flag
86d774420968 Add extended device capability flags
215736f7cdb1 Extend devinfo verbosity mode
102b4a1544d4 Add GID support to ibv_xsrq_pingpong example
d1a785654046 Fix exchange message size in ibv_xsrq_pinpong example
46cdea5373de Add an example to ibv_create_flow man page
69042c660f06 Add man page for flow steering verbs
c6eebd5adae5 Add support for don't trap steering rule
00b93fd20b28 Add MR re-registeration
8d97353b7857 Change rereg_mr API between libibverbs and the provider's
library
85061a41fb58 Extend man pages to describe Memory Window usage
398abfded7ea Expose device capabilities for Memory Window type two
568c67c878c2 Add bind/unbind support for Memory Window type two
6148efe68f5f Expose device capability for Memory Window type one
e437deed93c2 Add bind/unbind support for Memory Window type one
518fcbd42a29 Add alloc/dealloc Memory Window verbs
e29e171145b0 Update ibv_post_send manual page
5085d749c6bd Add QP creation flags, support blocking self multicast loopback
b5eac7cbfb0a Allow use of huge pages in multiple calls to ibv_fork_init
027dd68002ae libibverbs: Modify ibv_asyncwatch to accept the monitored
device
515a067586e0 Roll 1.2.0 release
89cb59ef7fd9 libibverbs.spec.in: Use config.h substitution
678f7fa7dde8 Makefile.am: Don't allow strict aliasing by default
70d200860584 Makefile.am: Fix "make distcheck"
700d098ed190 Add GID support to ibv_xsrq_pingpong example
fa87a514d257 Fix exchange message size in ibv_xsrq_pinpong example
21b0a974ed7c Add an example to ibv_create_flow man page
1755d907c523 Add man page for flow steering verbs
fa6574cfa3a2 Add support for don't trap steering rule
ba9b4ae0a9a3 Add MR re-registeration
ffb784b9514e Change rereg_mr API between libibverbs and the provider's
library
f224da05def5 Extend man pages to describe Memory Window usage
18f011121c70 Expose device capabilities for Memory Window type two
e45a827d01ec Add bind/unbind support for Memory Window type two
1f8c7f3c1f9d Expose device capability for Memory Window type one
6033c6986f12 Add bind/unbind support for Memory Window type one
fbf5740a75fb Add alloc/dealloc Memory Window verbs
f3e38c1d10c6 Update ibv_post_send manual page
0e60aab43bfb Add QP creation flags, support blocking self multicast loopback
2f9f51529f07 Allow use of huge pages in multiple calls to ibv_fork_init
786f5edc841f libibverbs: Modify ibv_asyncwatch to accept the monitored
device

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



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 884 bytes --]

Re: Upcoming libibverbs release

[Yishai Hadas]

On 6/23/2016 7:51 PM, Doug Ledford wrote:
> I'm preparing to make an official release of libibverbs soonish
> (thinking by the end of next week).  Even though I took the timestamp v5
> patchset, I know there were still comments from Jason on the v4 set that
> weren't addressed in v5.  Those can be addressed (and should be) with
> incremental patches prior to the official release.  Yishai, can we
> please prioritize getting that done?
>

Sure, will come up early next week with an answer for the opening
notes, considering their need and priority.
--
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: Upcoming libibverbs release

[Yishai Hadas]

On 6/23/2016 7:51 PM, Doug Ledford wrote:
> I'm preparing to make an official release of libibverbs soonish
> (thinking by the end of next week).  Even though I took the timestamp v5
> patchset, I know there were still comments from Jason on the v4 set that
> weren't addressed in v5.  Those can be addressed (and should be) with
> incremental patches prior to the official release.  Yishai, can we
> please prioritize getting that done?
>

Listed below the main openings that were raised by Jason and our input 
on, considering their need and priority.

Most of already answered as part of the discussion, will summarize it 
all together to help evaluating the status/next steps.


 > I'm still very much of the opinion that extended CQs should only allow
 > compatible QP's to be added.
--------------------------------------

Application may create a CQ with large subset of flags that may fit some 
of its attached QP types. Semantically, it is similar to what we have 
today, it allows to put different QP types on the same CQ and by thus 
allow application to have ordered completion events.

We don't see a good reason to change from current behavior and hurt 
applications that expect that valid behavior.

 > Add a compatibility layer
-------------------------------

A real application wants to achieve performance, to use a compatibility 
layer over a vendor that doesn’t support the new API, might be a very 
bad idea as of below.

1) The legacy API doesn't support the batching API that was introduced 
by the new API, this will lead to have a Door-Bell/PCI write per CQE as 
internally the compatibility layer will need to call ibv_poll_cq with 
size of 1.

2) There will 2 copies, one from the HW CQE to ibv_wc as part of the 
internal legacy API, second, from the ibv_wc to the read_xxx output, 
comparing the new API.

3) The *non-required* fields will be copied anyway as part of the 
internal legacy ibv_poll_cq from the HW CQE to ibv_wc and hits the 
performance.

In addition,
There is *no* one to one mapping in all the cases between the new API to 
the legacy one, it will lead to a *non* transparent usage of the 
compatibility layer and ends by returning an error to the application in 
few cases.

Specifically,
One of major benefits in the new API is the ability to say as part of CQ 
creation that it's going to be used by only one thread and have lock 
less flow as part of polling the CQ. This is not supported in the legacy 
API which assumes that multi-threaded access might be used.

It makes sense to return an error in the compatibility layer API letting 
application knows that this service is not supported when it was asked 
by the application.

Applications that look for performance expect to work in that single 
threaded mode and might fail as part of using the compatibility layer or 
alternatively have no idea that they didn't get the service which 
assumes to be bad behavior.

In addition, at the moment timestamping can’t be read via the legacy API 
means that the compatibility layer can’t handle it and should supply an 
error for. In the future there may be other fields/flows that can’t be 
supported by the compatibility layer so application that will be written 
over this layer will fail and won’t get the required benefit from using it.

To sum it up:
We don't see a real usage of such a layer, it has many limitations.
In case will be a real request from users, it can always be added in the 
future, minor priority for now.

 > Exact set of access flags
------------------------------

Application can ask for subset of the available bits, in addition, it 
has an option to ask for all legacy ones (i.e. IBV_WC_STANDARD_FLAGS) 
for a case that a CQ needs to support all legacy attribute and could be 
attached to both RC and UD QPs.

We didn't find it useful at the moment to add other enums per QP type as 
application can say explicitly what it wants by using the ORed bits and 
even per QP type application may not be always interested in all the 
fields. (e.g. CQ that used for TX only completions in RC is not 
interested in few fields that are relevant for the RX).

To sum it up:
Low priority issue that can be added in the future based on users demand.

 > Exact set of accessors
--------------------------------

At the moment the API introduces an option to get each field by a 
separate accessor (i.e ibv_read_wc_xxx), in addition, wr_id and status 
have direct access as application expect to use them in most of their flows.

During the discussion come up some ideas to add some extra accessors per 
QP type (e.g. ibv_wc_read_ud, ibv_wc_read_rc, etc.).

This option makes sense when application really plans to read and use 
all of the potential UD/RC fields, which *can't* be assumed as true in 
the general case.

Even in a UD/RC QP the application may be interested in few of the CQE 
fields while others are not needed.

 From bench mark testing, we found that gathering few fields in one 
ibv_wc_read_xxx has low benefit comparing the case that each field is 
read by itself.

In a case that *not* all fields are really needed the extra copy to 
return the redundant fields found to be more expensive comparing the 
ibv_wc_read_xxx when needed.

We can think of an application that may be interested in subset of 
fields per QP type or per send/receive flows to be gathered in one call, 
this ability can always be added in the future as the API cleanly
enables extensions.

If you still think that adding some extra ibv_wc_read_xxx groups from 
day one is really needed, let's define it and add now by an incremental 
patch. Till now didn't see any user specific request for.

 > Minor coding issues and micro optimizations
------------------------------------------------

Not sure what exactly that point refers to from API point of view, 
assuming that it relates to few comments given on libmlx5 as of below.

 > +static inline uint32_t mlx5_cq_read_wc_qp_num(struct ibv_cq_ex *ibcq)
 > +{
 > +	struct mlx5_cq *cq = to_mcq(ibv_cq_ex_to_cq(ibcq));

 > You might want to look at having the caller pass in 'cq' from a member
 > in ibv_cq_ex. That way the caller can hold the cq in a register
 > instead of constantly reloading it like this - I'm assuming to_mcq is
 > not just an container_of??

The assumption is *incorrect*, the internal access from ibv_cq_ex to 
driver CQ is done via offsetof/container_of which is done in compile 
time, no need to have some change from the clean usage of the API to 
that mode.


 > +static inline uint32_t mlx5_cq_read_wc_qp_num(struct ibv_cq_ex
 > *ibcq)
 > You need to go through everything you've written and make sure it is
 > const-correct.
 > You should also annotate with restrict.
 > Doing this properly can increase performance by allowing the caller to
 > avoid reloads.

 From performance point of view we don't expect a change here, we 
followed the created assembly code and saw no difference even when it 
was annotated with 'restrict'.

In addition, please note the 'restrict' notation was only became 
standard as part of C99, in previous compilers it may not work at all or 
requires different key word to be defined as part of the CONFIG step.

We can go ahead and define the ibv_wc_read_xxx with const qualifier on 
its input struct ibv_cq_ex *ibcq in some extra incremental patch if 
makes sense.

--
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: Upcoming libibverbs release

[Jason Gunthorpe]

On Sun, Jun 26, 2016 at 07:54:21PM +0300, Yishai Hadas wrote:
> > I'm still very much of the opinion that extended CQs should only allow
> > compatible QP's to be added.
> 
> Application may create a CQ with large subset of flags that may fit some of
> its attached QP types. Semantically, it is similar to what we have today, it
> allows to put different QP types on the same CQ and by thus allow
> application to have ordered completion events.

This is repeating your argument, I disagree, it is a horrible API
design, you need to defend this with an actual use case.

> We don't see a good reason to change from current behavior and hurt
> applications that expect that valid behavior.

Safety for the API consumer.

> > Add a compatibility layer
> 
> A real application wants to achieve performance, to use a compatibility
> layer over a vendor that doesn???t support the new API, might be a very bad
> idea as of below.

Well, NAK on this from me then.

Doug and I have both stated we don't want to see so many
single-provider APIs and churn in libibverbs. This was designed to be
a generic API that all providers can implement.

The responsibility falls on you to make sure that it works
universally. Compat is one option.

This is also why it is necessary to get the other provider authors to
say this works on their hardware, because they *all* ultimately have
to implement it.

I don't think many people realize this yet.

> > Exact set of access flags
> 
> Application can ask for subset of the available bits, in addition, it has an
> option to ask for all legacy ones (i.e. IBV_WC_STANDARD_FLAGS) for a case
> that a CQ needs to support all legacy attribute and could be attached to
> both RC and UD QPs.

Again, this is pretty much nonsense. Asking for data that can't be
returned is just an insane API design.

> Low priority issue that can be added in the future based on users demand.

No it can't, the enum and flags are part of the API and have to be
designed correctly from the start.

> From bench mark testing, we found that gathering few fields in one
> ibv_wc_read_xxx has low benefit comparing the case that each field is read
> by itself.

This might be a good argument.

> If you still think that adding some extra ibv_wc_read_xxx groups from day
> one is really needed, let's define it and add now by an incremental patch.
> Till now didn't see any user specific request for.

What? 'user request' for a just designed API? How is that even
possible?

> > Minor coding issues and micro optimizations
> 
> Not sure what exactly that point refers to from API point of view, assuming
> that it relates to few comments given on libmlx5 as of below.

I vaugely remember there being lots of comments, you guys basically
abandoned the whole discussion once Doug merged it, so I don't really
recall anymore. 

> > +static inline uint32_t mlx5_cq_read_wc_qp_num(struct ibv_cq_ex *ibcq)
> > +{
> > +	struct mlx5_cq *cq = to_mcq(ibv_cq_ex_to_cq(ibcq));
> 
> > You might want to look at having the caller pass in 'cq' from a member
> > in ibv_cq_ex. That way the caller can hold the cq in a register
> > instead of constantly reloading it like this - I'm assuming to_mcq is
> > not just an container_of??
> 
> The assumption is *incorrect*, the internal access from ibv_cq_ex to driver
> CQ is done via offsetof/container_of which is done in compile time, no need
> to have some change from the clean usage of the API to that mode.

Fine on cq, but the wqe pointer may benifit, and other drivers may
benifit. It still needs to be studied.

> > *ibcq)
> > You need to go through everything you've written and make sure it is
> > const-correct.
> > You should also annotate with restrict.
> > Doing this properly can increase performance by allowing the caller to
> > avoid reloads.
> 
> From performance point of view we don't expect a change here, we followed
> the created assembly code and saw no difference even when it was annotated
> with 'restrict'.

The annotations help the caller.

> In addition, please note the 'restrict' notation was only became standard as
> part of C99, in previous compilers it may not work at all or requires
> different key word to be defined as part of the CONFIG step.

Use __restrict like glibc does.

> We can go ahead and define the ibv_wc_read_xxx with const qualifier on its
> input struct ibv_cq_ex *ibcq in some extra incremental patch if makes sense.

Of course it makes sense, you need to fix all of this stuff in all
the patches you wrote.

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: Upcoming libibverbs release

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 868 bytes --]

On Mon, Jun 27, 2016 at 12:17:58PM -0600, Jason Gunthorpe wrote:
> Doug and I have both stated we don't want to see so many
> single-provider APIs and churn in libibverbs. This was designed to be
> a generic API that all providers can implement.
> 
> The responsibility falls on you to make sure that it works
> universally. Compat is one option.
> 
> This is also why it is necessary to get the other provider authors to
> say this works on their hardware, because they *all* ultimately have
> to implement it.
> 
> I don't think many people realize this yet.
> 

Jason,
You are over-estimating the number of other providers who are
interesting in libibverbs in 2014-2016.

According to git log [1] in these years, I see only one major
contributor to this library.

[1] https://git.kernel.org/cgit/libs/infiniband/libibverbs.git/log/

Thanks

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: Upcoming libibverbs release

[Knut Omang]

On Tue, 2016-06-28 at 08:02 +0300, Leon Romanovsky wrote:
> On Mon, Jun 27, 2016 at 12:17:58PM -0600, Jason Gunthorpe wrote:
> > Doug and I have both stated we don't want to see so many
> > single-provider APIs and churn in libibverbs. This was designed to
> > be
> > a generic API that all providers can implement.
> > 
> > The responsibility falls on you to make sure that it works
> > universally. Compat is one option.
> > 
> > This is also why it is necessary to get the other provider authors
> > to
> > say this works on their hardware, because they *all* ultimately
> > have
> > to implement it.
> > 
> > I don't think many people realize this yet.
> > 
> 
> Jason,
> You are over-estimating the number of other providers who are
> interesting in libibverbs in 2014-2016.
> 
> According to git log [1] in these years, I see only one major
> contributor to this library.

I appreciate Jason's concern for other providers, I can assure we both
use libibverbs for the new Oracle HCAs and also have changes/fixes to
it. I want to get them out here, just a matter of getting enough time
set aside to catch up (with the recent churn on both sides of the
kernel/user boundary)

> [1] https://git.kernel.org/cgit/libs/infiniband/libibverbs.git/log/

Thanks,

Knut

-- 
Knut Omang, Ph.D
Principal Software Engineer
Oracle NSN Europe Design Center



--
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: Upcoming libibverbs release

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 1762 bytes --]

On Tue, Jun 28, 2016 at 05:52:13PM +0200, Knut Omang wrote:
> On Tue, 2016-06-28 at 08:02 +0300, Leon Romanovsky wrote:
> > On Mon, Jun 27, 2016 at 12:17:58PM -0600, Jason Gunthorpe wrote:
> > > Doug and I have both stated we don't want to see so many
> > > single-provider APIs and churn in libibverbs. This was designed to
> > > be
> > > a generic API that all providers can implement.
> > > 
> > > The responsibility falls on you to make sure that it works
> > > universally. Compat is one option.
> > > 
> > > This is also why it is necessary to get the other provider authors
> > > to
> > > say this works on their hardware, because they *all* ultimately
> > > have
> > > to implement it.
> > > 
> > > I don't think many people realize this yet.
> > > 
> > 
> > Jason,
> > You are over-estimating the number of other providers who are
> > interesting in libibverbs in 2014-2016.
> > 
> > According to git log [1] in these years, I see only one major
> > contributor to this library.
> 
> I appreciate Jason's concern for other providers, I can assure we both
> use libibverbs for the new Oracle HCAs and also have changes/fixes to
> it. I want to get them out here, just a matter of getting enough time
> set aside to catch up (with the recent churn on both sides of the
> kernel/user boundary)

It is great and we all be happy to see more players in that field.
However it is not enough to use this library, Jason wants to see commitment
from other vendors to be ready to implement new proposed features.

> 
> > [1] https://git.kernel.org/cgit/libs/infiniband/libibverbs.git/log/
> 
> Thanks,
> 
> Knut
> 
> -- 
> Knut Omang, Ph.D
> Principal Software Engineer
> Oracle NSN Europe Design Center
> 
> 
> 

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: Upcoming libibverbs release

[Jason Gunthorpe]

On Tue, Jun 28, 2016 at 08:02:46AM +0300, Leon Romanovsky wrote:
> On Mon, Jun 27, 2016 at 12:17:58PM -0600, Jason Gunthorpe wrote:
> > Doug and I have both stated we don't want to see so many
> > single-provider APIs and churn in libibverbs. This was designed to be
> > a generic API that all providers can implement.
> > 
> > The responsibility falls on you to make sure that it works
> > universally. Compat is one option.
> > 
> > This is also why it is necessary to get the other provider authors to
> > say this works on their hardware, because they *all* ultimately have
> > to implement it.
> > 
> > I don't think many people realize this yet.
> > 
> 
> Jason,
> You are over-estimating the number of other providers who are
> interesting in libibverbs in 2014-2016.

I don't think that is a reasonable metric, verbs is stable, I don't
expect companies to be constantly churning it.

And that certainly doesn't mean we should abandon their providers when
building new APIs.

However, if that is your position then propose a patch so libibverbs
with the new polling API will not load old providers that do not
support it, and they can be deprecated. If their authors don't object,
like you predict, then great.

But at the end of the day, the message to users must be to use the
new polling API, the old one is deprecated, and apps should never
include fallback code because the new API always works.

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: Upcoming libibverbs release

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 1856 bytes --]

On Tue, Jun 28, 2016 at 10:20:28AM -0600, Jason Gunthorpe wrote:
> On Tue, Jun 28, 2016 at 08:02:46AM +0300, Leon Romanovsky wrote:
> > On Mon, Jun 27, 2016 at 12:17:58PM -0600, Jason Gunthorpe wrote:
> > > Doug and I have both stated we don't want to see so many
> > > single-provider APIs and churn in libibverbs. This was designed to be
> > > a generic API that all providers can implement.
> > > 
> > > The responsibility falls on you to make sure that it works
> > > universally. Compat is one option.
> > > 
> > > This is also why it is necessary to get the other provider authors to
> > > say this works on their hardware, because they *all* ultimately have
> > > to implement it.
> > > 
> > > I don't think many people realize this yet.
> > > 
> > 
> > Jason,
> > You are over-estimating the number of other providers who are
> > interesting in libibverbs in 2014-2016.
> 
> I don't think that is a reasonable metric, verbs is stable, I don't
> expect companies to be constantly churning it.
> 
> And that certainly doesn't mean we should abandon their providers when
> building new APIs.
> 
> However, if that is your position then propose a patch so libibverbs
> with the new polling API will not load old providers that do not
> support it, and they can be deprecated. If their authors don't object,
> like you predict, then great.
> 
> But at the end of the day, the message to users must be to use the
> new polling API, the old one is deprecated, and apps should never
> include fallback code because the new API always works.

Please put aside Yishai's patches, I'm not taking about them.

We are talking about the total number of other vendors who will
be ready to implement new features exposed in libibverbs.

Git log perfectly supports my claim that this number is extremely low.

> 
> Jason

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

RE: Upcoming libibverbs release

[Steve Wise]

> On Tue, Jun 28, 2016 at 10:20:28AM -0600, Jason Gunthorpe wrote:
> > On Tue, Jun 28, 2016 at 08:02:46AM +0300, Leon Romanovsky wrote:
> > > On Mon, Jun 27, 2016 at 12:17:58PM -0600, Jason Gunthorpe wrote:
> > > > Doug and I have both stated we don't want to see so many
> > > > single-provider APIs and churn in libibverbs. This was designed to be
> > > > a generic API that all providers can implement.
> > > >
> > > > The responsibility falls on you to make sure that it works
> > > > universally. Compat is one option.
> > > >
> > > > This is also why it is necessary to get the other provider authors to
> > > > say this works on their hardware, because they *all* ultimately have
> > > > to implement it.
> > > >
> > > > I don't think many people realize this yet.
> > > >
> > >
> > > Jason,
> > > You are over-estimating the number of other providers who are
> > > interesting in libibverbs in 2014-2016.
> >
> > I don't think that is a reasonable metric, verbs is stable, I don't
> > expect companies to be constantly churning it.
> >
> > And that certainly doesn't mean we should abandon their providers when
> > building new APIs.
> >
> > However, if that is your position then propose a patch so libibverbs
> > with the new polling API will not load old providers that do not
> > support it, and they can be deprecated. If their authors don't object,
> > like you predict, then great.
> >
> > But at the end of the day, the message to users must be to use the
> > new polling API, the old one is deprecated, and apps should never
> > include fallback code because the new API always works.
> 
> Please put aside Yishai's patches, I'm not taking about them.
> 
> We are talking about the total number of other vendors who will
> be ready to implement new features exposed in libibverbs.
> 
> Git log perfectly supports my claim that this number is extremely low.
> 

I haven't been following this (long) thread closely enough, but I hope any new
functionality is not _breaking_ existing applications from using _currently
supported_ providers?  I thought all this CQ accessor cruft was backwards
compatible.  Perhaps I'm wrong?





--
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: Upcoming libibverbs release

[Jason Gunthorpe]

On Tue, Jun 28, 2016 at 02:12:19PM -0500, Steve Wise wrote:

> I haven't been following this (long) thread closely enough, but I hope any new
> functionality is not _breaking_ existing applications from using _currently
> supported_ providers?  I thought all this CQ accessor cruft was backwards
> compatible.  Perhaps I'm wrong?

As merged, the new API simply does not work without provider support.

The old API keeps working of course.

The fundamental question is if it is acceptable for libibverbs to ship
a new core provider agnostic API (cq polling) that is optional
depending on provider.

I say no, that is not acceptable.

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: Upcoming libibverbs release

[Hefty, Sean]

> The fundamental question is if it is acceptable for libibverbs to ship
> a new core provider agnostic API (cq polling) that is optional
> depending on provider.
> 
> I say no, that is not acceptable.

I agree - there's no compelling argument that the new calls can't work over all providers.
--
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: Upcoming libibverbs release

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 475 bytes --]

On Tue, Jun 28, 2016 at 09:26:38PM +0000, Hefty, Sean wrote:
> > The fundamental question is if it is acceptable for libibverbs to ship
> > a new core provider agnostic API (cq polling) that is optional
> > depending on provider.
> > 
> > I say no, that is not acceptable.
> 
> I agree - there's no compelling argument that the new calls can't work over all providers.

I agree too, this is why code is provided and nothing stops from other provider to support it.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: Upcoming libibverbs release

[Jason Gunthorpe]

On Wed, Jun 29, 2016 at 08:19:56AM +0300, Leon Romanovsky wrote:
> On Tue, Jun 28, 2016 at 09:26:38PM +0000, Hefty, Sean wrote:
> > > The fundamental question is if it is acceptable for libibverbs to ship
> > > a new core provider agnostic API (cq polling) that is optional
> > > depending on provider.
> > > 
> > > I say no, that is not acceptable.
> > 
> > I agree - there's no compelling argument that the new calls can't work over all providers.
> 
> I agree too, this is why code is provided and nothing stops from other provider to support it.

That isn't what I said, and it isn't what everyone is agreeing with.

I said an optinal API is not acceptable to ship.

That means 'nothing stops' is not enough - the patch author and
maintainer have the responsibility to ensure all providers work with
the API before shipping it.

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: Upcoming libibverbs release

[Liran Liss]

> From: linux-rdma-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org [mailto:linux-rdma-
> owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org] On Behalf Of Jason Gunthorpe

> On Wed, Jun 29, 2016 at 08:19:56AM +0300, Leon Romanovsky wrote:
> > On Tue, Jun 28, 2016 at 09:26:38PM +0000, Hefty, Sean wrote:
> > > > The fundamental question is if it is acceptable for libibverbs to
> > > > ship a new core provider agnostic API (cq polling) that is
> > > > optional depending on provider.
> > > >
> > > > I say no, that is not acceptable.
> > >
> > > I agree - there's no compelling argument that the new calls can't work over
> all providers.
> >
> > I agree too, this is why code is provided and nothing stops from other provider
> to support it.
> 
> That isn't what I said, and it isn't what everyone is agreeing with.
> 
> I said an optinal API is not acceptable to ship.
> 

What?
Almost every device interface has optional APIs.

The most immediate example is our beloved netdevice, in which almost everything is optional: fcoe support, flow steering, fdb settings, sriov controls...
All of these features are provider-agnostic, yet each netdev either implements them or not.

Especially in a performance-critical interface a polling API, I think that it actually makes more sense that each vendor provides the most optimized implementation that suits the HW.
--Liran

--
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: Upcoming libibverbs release

[Jason Gunthorpe]

On Thu, Jun 30, 2016 at 06:17:06PM +0000, Liran Liss wrote:
> > From: linux-rdma-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org [mailto:linux-rdma-
> > owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org] On Behalf Of Jason Gunthorpe
> 
> > On Wed, Jun 29, 2016 at 08:19:56AM +0300, Leon Romanovsky wrote:
> > > On Tue, Jun 28, 2016 at 09:26:38PM +0000, Hefty, Sean wrote:
> > > > > The fundamental question is if it is acceptable for libibverbs to
> > > > > ship a new core provider agnostic API (cq polling) that is
> > > > > optional depending on provider.
> > > > >
> > > > > I say no, that is not acceptable.
> > > >
> > > > I agree - there's no compelling argument that the new calls can't work over
> > all providers.
> > >
> > > I agree too, this is why code is provided and nothing stops from other provider
> > to support it.
> > 
> > That isn't what I said, and it isn't what everyone is agreeing with.
> > 
> > I said an optinal API is not acceptable to ship.
> > 
> 
> What?
> Almost every device interface has optional APIs.

re-read what I said. I'm not talking generally, but specifically about
this case.

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: Upcoming libibverbs release

[Doug Ledford]

[-- Attachment #1.1: Type: text/plain, Size: 2147 bytes --]

On 6/29/2016 2:30 PM, Jason Gunthorpe wrote:
> On Wed, Jun 29, 2016 at 08:19:56AM +0300, Leon Romanovsky wrote:
>> On Tue, Jun 28, 2016 at 09:26:38PM +0000, Hefty, Sean wrote:
>>>> The fundamental question is if it is acceptable for libibverbs to ship
>>>> a new core provider agnostic API (cq polling) that is optional
>>>> depending on provider.
>>>>
>>>> I say no, that is not acceptable.
>>>
>>> I agree - there's no compelling argument that the new calls can't work over all providers.
>>
>> I agree too, this is why code is provided and nothing stops from other provider to support it.
> 
> That isn't what I said, and it isn't what everyone is agreeing with.
> 
> I said an optinal API is not acceptable to ship.
> 
> That means 'nothing stops' is not enough - the patch author and
> maintainer have the responsibility to ensure all providers work with
> the API before shipping it.

That's not entirely true.  In the kernel, if someone cuts over stuff
from one API to a replacement API, then yes, they are responsible for
updating all of the users of the old API.  But if they create a new API
that is in addition to the old API, then that's not the case.  This
falls in that latter category.

That said, there is no reason that owners of various driver libraries
can't update their own libraries to this API.  As we already discussed
in another thread when Steve Wise brought up the issue of how things
would look if we put the providers into the same package as libibverbs,
distros and such have their own schedules and the day we release
something here is not the same day they pick things up there, so we
should have sufficient time to update libibverbs and then cut over the
driver modules.  They don't have to happen on the same day.

What I don't want is a core compat layer in libibverbs.  The changes
needed to support just the pull cq changes are not that drastic and they
have benefit that would be lost in a compat layer, so I would rather see
them done natively on a per library basis.

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 884 bytes --]

Re: Upcoming libibverbs release

[Jason Gunthorpe]

On Tue, Jul 19, 2016 at 03:57:03PM -0400, Doug Ledford wrote:
> On 6/29/2016 2:30 PM, Jason Gunthorpe wrote:
> > On Wed, Jun 29, 2016 at 08:19:56AM +0300, Leon Romanovsky wrote:
> >> On Tue, Jun 28, 2016 at 09:26:38PM +0000, Hefty, Sean wrote:
> >>>> The fundamental question is if it is acceptable for libibverbs to ship
> >>>> a new core provider agnostic API (cq polling) that is optional
> >>>> depending on provider.
> >>>>
> >>>> I say no, that is not acceptable.
> >>>
> >>> I agree - there's no compelling argument that the new calls can't work over all providers.
> >>
> >> I agree too, this is why code is provided and nothing stops from other provider to support it.
> > 
> > That isn't what I said, and it isn't what everyone is agreeing with.
> > 
> > I said an optinal API is not acceptable to ship.
> > 
> > That means 'nothing stops' is not enough - the patch author and
> > maintainer have the responsibility to ensure all providers work with
> > the API before shipping it.
> 
> That's not entirely true.  In the kernel, if someone cuts over stuff
> from one API to a replacement API, then yes, they are responsible for
> updating all of the users of the old API.  But if they create a new API
> that is in addition to the old API, then that's not the case.  This
> falls in that latter category.

This isn't the kernel. This is a user facing library that has to be
explained to end-developers.

Telling them to use API A if they detect mlx5 and API B otherwise is
madness, that might fly in the kernel, but it makes no sense
for libibverbs.

> That said, there is no reason that owners of various driver libraries
> can't update their own libraries to this API.  As we already discussed
> in another thread when Steve Wise brought up the issue of how things

We don't even have a single other implementation, not even mthca and
mlx4 that Mellanox is responsible for and it has been a long time
now..

> something here is not the same day they pick things up there, so we
> should have sufficient time to update libibverbs and then cut over the
> driver modules.  They don't have to happen on the same day.

Sure, but will that ever happen??

> What I don't want is a core compat layer in libibverbs.  The changes
> needed to support just the pull cq changes are not that drastic and they
> have benefit that would be lost in a compat layer, so I would rather see
> them done natively on a per library basis.

The benifit is not performance, it is a uniform single API that apps
can code against. If new apps are being run on older hardware then
then vendor should be motivated to natively implement the API. But we
should never tell an app writer they cannot use a core API because of
the driver.

The patch that was given to the ping example is is exactly the
horrible situation I do not want to put our app authors in, completely
duplicating CQ processing in the app makes no sense.

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: Upcoming libibverbs release

[Jason Gunthorpe]

On Tue, Jun 28, 2016 at 08:05:49PM +0300, Leon Romanovsky wrote:

> > But at the end of the day, the message to users must be to use the
> > new polling API, the old one is deprecated, and apps should never
> > include fallback code because the new API always works.
> 
> Please put aside Yishai's patches, I'm not taking about them.

Well, I am, I don't know what you are talking about.

> We are talking about the total number of other vendors who will
> be ready to implement new features exposed in libibverbs.

Who cares?

Mellanox has been pushing dpdk centric HW features that no other
vendor has an interest in. So it is understandable there is not much
activity.

However this is an all-vendor software only feature that all HW can
support right now today, it is fundamentally different than the prior
HW entangled patches.

Consider the kAPI work to add the new MR stuff that Christoph
did. That would have been a total disaster if they didn't update all
the drivers to work with it at the same time.

Userspace is no different, and the responsibility falls with the patch
author to oversee that process and get it done before merging.

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: Upcoming libibverbs release

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 1915 bytes --]

On Tue, Jun 28, 2016 at 03:18:58PM -0600, Jason Gunthorpe wrote:
> On Tue, Jun 28, 2016 at 08:05:49PM +0300, Leon Romanovsky wrote:
> 
> > > But at the end of the day, the message to users must be to use the
> > > new polling API, the old one is deprecated, and apps should never
> > > include fallback code because the new API always works.
> > 
> > Please put aside Yishai's patches, I'm not taking about them.
> 
> Well, I am, I don't know what you are talking about.
> 
> > We are talking about the total number of other vendors who will
> > be ready to implement new features exposed in libibverbs.
> 
> Who cares?
> 
> Mellanox has been pushing dpdk centric HW features that no other
> vendor has an interest in. So it is understandable there is not much
> activity.

I see it differently, I don't see any activity from other vendors,
because there are no vendors who interest in developing new features in
libibverbs.

Currently all vendors can be categorized into three groups:
* Interested in legacy code - a couple of vendors
* Struggling from identity problem (verbs/non-verbs architecture) and
  has proprietary library - one vendor
* Interested in developing new features in libibverbs - one vendor

> 
> However this is an all-vendor software only feature that all HW can
> support right now today, it is fundamentally different than the prior
> HW entangled patches.
> 
> Consider the kAPI work to add the new MR stuff that Christoph
> did. That would have been a total disaster if they didn't update all
> the drivers to work with it at the same time.
> 
> Userspace is no different, and the responsibility falls with the patch
> author to oversee that process and get it done before merging.

Again, my response is related to your expectations to see "other
vendors". Please don't drag me to the discussion of Yishai's patches.

Thanks.

> 
> Jason

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: Upcoming libibverbs release

[Knut Omang]

On Wed, 2016-06-29 at 08:15 +0300, Leon Romanovsky wrote:
> On Tue, Jun 28, 2016 at 03:18:58PM -0600, Jason Gunthorpe wrote:
> > On Tue, Jun 28, 2016 at 08:05:49PM +0300, Leon Romanovsky wrote:
> > 
> > > > But at the end of the day, the message to users must be to use the
> > > > new polling API, the old one is deprecated, and apps should never
> > > > include fallback code because the new API always works.
> > > 
> > > Please put aside Yishai's patches, I'm not taking about them.
> > 
> > Well, I am, I don't know what you are talking about.
> > 
> > > We are talking about the total number of other vendors who will
> > > be ready to implement new features exposed in libibverbs.
> > 
> > Who cares?
> > 
> > Mellanox has been pushing dpdk centric HW features that no other
> > vendor has an interest in. So it is understandable there is not much
> > activity.
> 
> I see it differently, I don't see any activity from other vendors,
> because there are no vendors who interest in developing new features in
> libibverbs.

A fundamental property of good APIs is that they provide a certain
degree of stability and backward/forward compatibility. This means (in
my view) that for an API which have reached a certain maturity and user
base, "default" action should be not to change, and that changes must
have compelling arguments. That said, good additions and necessary
refactoring is fine, as long as enough care is taken to maintain
backward compatibility or a well understood, viable upgrade path for
the existing user base.

> Currently all vendors can be categorized into three groups:
> * Interested in legacy code - a couple of vendors
> * Struggling from identity problem (verbs/non-verbs architecture) and
>   has proprietary library - one vendor
> * Interested in developing new features in libibverbs - one vendor

I would think that we would want to eventually implement new libibverbs
features that makes sense from an application point of view, with
priority on those that we believe are most important to our users.

My point is just that making changes to libibverbs should be motivated
by use cases that cannot be covered by the existing API, or that is
clumsy or inefficient or not generic enough, and not be accepted until
they have been subject to enough scrutiny to ensure that quality.

Through the course of my work with the Infiniband stack so far, I have
seen a few cases of changes that were not of that sort, so I appreciate
that the community strives to make sure that the "burden of proof" is
on the side of those who want changes.

Thanks,
Knut

> > 
> > However this is an all-vendor software only feature that all HW can
> > support right now today, it is fundamentally different than the prior
> > HW entangled patches.
> > 
> > Consider the kAPI work to add the new MR stuff that Christoph
> > did. That would have been a total disaster if they didn't update all
> > the drivers to work with it at the same time.
> > 
> > Userspace is no different, and the responsibility falls with the patch
> > author to oversee that process and get it done before merging.
> 
> Again, my response is related to your expectations to see "other
> vendors". Please don't drag me to the discussion of Yishai's patches.
> 
> Thanks.
> 
> > 
> > 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: Upcoming libibverbs release

[Yishai Hadas]

On 6/29/2016 8:07 PM, Knut Omang wrote:
> On Wed, 2016-06-29 at 08:15 +0300, Leon Romanovsky wrote:
>> On Tue, Jun 28, 2016 at 03:18:58PM -0600, Jason Gunthorpe wrote:
>>> On Tue, Jun 28, 2016 at 08:05:49PM +0300, Leon Romanovsky wrote:
>>>
>>>>> But at the end of the day, the message to users must be to use the
>>>>> new polling API, the old one is deprecated, and apps should never
>>>>> include fallback code because the new API always works.
>>>>
>>>> Please put aside Yishai's patches, I'm not taking about them.
>>>
>>> Well, I am, I don't know what you are talking about.
>>>
>>>> We are talking about the total number of other vendors who will
>>>> be ready to implement new features exposed in libibverbs.
>>>
>>> Who cares?
>>>
>>> Mellanox has been pushing dpdk centric HW features that no other
>>> vendor has an interest in. So it is understandable there is not much
>>> activity.
>>
>> I see it differently, I don't see any activity from other vendors,
>> because there are no vendors who interest in developing new features in
>> libibverbs.
>
> A fundamental property of good APIs is that they provide a certain
> degree of stability and backward/forward compatibility. This means (in
> my view) that for an API which have reached a certain maturity and user
> base, "default" action should be not to change, and that changes must
> have compelling arguments.

The legacy API wasn't changed, no plan to break/change it.

That said, good additions and necessary
> refactoring is fine, as long as enough care is taken to maintain
> backward compatibility or a well understood, viable upgrade path for
> the existing user base.
>

There was a need to extend the output data that comes out from a vendor 
CQE (e.g. get timestamping). Current API wasn't written in an extended 
mode, that was the motivation to look for a new/better API.

After a long discussion in the list we came with a new API which is 
extendable, support legacy functionality, generic and easy to understand 
and use.

In addition, the API was introduced in some forums (OFA last summit, 
verbs working group) and was found to be make sense that can fit 
multi-vendors.

>> Currently all vendors can be categorized into three groups:
>> * Interested in legacy code - a couple of vendors
>> * Struggling from identity problem (verbs/non-verbs architecture) and
>>   has proprietary library - one vendor
>> * Interested in developing new features in libibverbs - one vendor
>
> I would think that we would want to eventually implement new libibverbs
> features that makes sense from an application point of view, with
> priority on those that we believe are most important to our users.
>
> My point is just that making changes to libibverbs should be motivated
> by use cases that cannot be covered by the existing API,

See motivation for the new API above in my note.


> Through the course of my work with the Infiniband stack so far, I have
> seen a few cases of changes that were not of that sort, so I appreciate
> that the community strives to make sure that the "burden of proof" is
> on the side of those who want changes.

We already supplied "burden of proof" with CX4 driver, vendor code in 
libmlx5 was published and merged, see report in the list.



> Thanks,
> Knut
>
>>>
>>> However this is an all-vendor software only feature that all HW can
>>> support right now today, it is fundamentally different than the prior
>>> HW entangled patches.
>>>
>>> Consider the kAPI work to add the new MR stuff that Christoph
>>> did. That would have been a total disaster if they didn't update all
>>> the drivers to work with it at the same time.
>>>
>>> Userspace is no different, and the responsibility falls with the patch
>>> author to oversee that process and get it done before merging.
>>
>> Again, my response is related to your expectations to see "other
>> vendors". Please don't drag me to the discussion of Yishai's patches.
>>
>> Thanks.
>>
>>>
>>> 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: Upcoming libibverbs release

[Christoph Hellwig]

On Tue, Jun 28, 2016 at 03:18:58PM -0600, Jason Gunthorpe wrote:
> Consider the kAPI work to add the new MR stuff that Christoph
> did. That would have been a total disaster if they didn't update all
> the drivers to work with it at the same time.
> 
> Userspace is no different, and the responsibility falls with the patch
> author to oversee that process and get it done before merging.

Yes.  I think we need to a) put all userspace providers into the
libibverbs repository, and b) require patches to update all of them.
That's the only way to have coherent releases and avoid bitrot.
--
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: Upcoming libibverbs release

[Jason Gunthorpe]

On Wed, Jun 29, 2016 at 05:09:20AM -0700, Christoph Hellwig wrote:
> On Tue, Jun 28, 2016 at 03:18:58PM -0600, Jason Gunthorpe wrote:
> > Consider the kAPI work to add the new MR stuff that Christoph
> > did. That would have been a total disaster if they didn't update all
> > the drivers to work with it at the same time.
> > 
> > Userspace is no different, and the responsibility falls with the patch
> > author to oversee that process and get it done before merging.
> 
> Yes.  I think we need to a) put all userspace providers into the
> libibverbs repository, and b) require patches to update all of them.
> That's the only way to have coherent releases and avoid bitrot.

Yep, that would be great.

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: Upcoming libibverbs release

[Steve Wise]

> On Wed, Jun 29, 2016 at 05:09:20AM -0700, Christoph Hellwig wrote:
> > On Tue, Jun 28, 2016 at 03:18:58PM -0600, Jason Gunthorpe wrote:
> > > Consider the kAPI work to add the new MR stuff that Christoph
> > > did. That would have been a total disaster if they didn't update all
> > > the drivers to work with it at the same time.
> > >
> > > Userspace is no different, and the responsibility falls with the patch
> > > author to oversee that process and get it done before merging.
> >
> > Yes.  I think we need to a) put all userspace providers into the
> > libibverbs repository, and b) require patches to update all of them.
> > That's the only way to have coherent releases and avoid bitrot.
> 
> Yep, that would be great.
> 
> Jason

I have a concern about the loss of control that might happen.  For example, now
I can release a new libcxgb4 whenever it works for me.  I wouldn't want to give
that up and require everyone's blessing/coordination to release bug fixes that
are completely in libcxgb4.  How can we handle this with a single repo for all
the provider libs?

Steve.

--
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: Upcoming libibverbs release

[Jason Gunthorpe]

On Wed, Jun 29, 2016 at 01:46:16PM -0500, Steve Wise wrote:

> I have a concern about the loss of control that might happen.  For example, now
> I can release a new libcxgb4 whenever it works for me.  I wouldn't want to give
> that up and require everyone's blessing/coordination to release bug fixes that
> are completely in libcxgb4.  How can we handle this with a single repo for all
> the provider libs?

I think you start by realizing that nobody uses your git releases
anyhow.

Everyone uses either a distro or OFED release and those are already
co-ordinated outside your control.

Changing the place the distro gets the patches from makes no real
difference to you or your users. However, it does make the distro's
life simpler by having only one place to look for patches.

I'd expect that provider-only patches would have a fast path into the
combined git head, since they don't really need any consensus, and
that is enough to get them into the backport/release cycle for the
above channels.

Presumably Doug would be careful to have stable and devel heads in the
repo where provider fixes vs API changes can land.

Maybe Leon would think more people care if he saw provider commits in
the libibverbs <shrug>

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: Upcoming libibverbs release

[Steve Wise]

> 
> > I have a concern about the loss of control that might happen.  For example,
now
> > I can release a new libcxgb4 whenever it works for me.  I wouldn't want to
give
> > that up and require everyone's blessing/coordination to release bug fixes
that
> > are completely in libcxgb4.  How can we handle this with a single repo for
all
> > the provider libs?
> 
> I think you start by realizing that nobody uses your git releases
> anyhow.
> 

Not true.  RedHat, for one, does.

> Everyone uses either a distro or OFED release and those are already
> co-ordinated outside your control.

I don't know who "everyone" is, but at least some of the distros get the
libcxgb3/4 packages from releases I publish and announce on linux-rdma.  Chelsio
recommends to RedHat exactly which libcxb release to pull in for each of their
OS releases.  They are not necessarily tied to an OFED release either.   Ditto
for SUSE, although they may still take from OFED packages.

> 
> Changing the place the distro gets the patches from makes no real
> difference to you or your users. However, it does make the distro's
> life simpler by having only one place to look for patches.
> 

>From what I've experienced, the distros don't take patches for libraries at all.
They take discreet releases that are published by the maintainer of that
library.  Doug, correct me if I'm wrong.  But that is how things are working
today at least for libcxgb*.   This is different from the kernel, where distros
pull in commits for fixes.

> I'd expect that provider-only patches would have a fast path into the
> combined git head, since they don't really need any consensus, and
> that is enough to get them into the backport/release cycle for the
> above channels.
>

Yes, and I'm asking that we figure this out before we cram all these libraries
into one git repo.  And again, currently I think distros pull release tarballs
and not from any git trees for the rdma provider libraries...
 
Steve.

--
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: Upcoming libibverbs release

[Hefty, Sean]

> Yes, and I'm asking that we figure this out before we cram all these
> libraries
> into one git repo.  And again, currently I think distros pull release
> tarballs
> and not from any git trees for the rdma provider libraries...

A git repo can still generate multiple packages/libraries.

But, even if you didn't have this, this would just follow the same release procedure that is used by the kernel.
--
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: Upcoming libibverbs release

[Steve Wise]

> > Yes, and I'm asking that we figure this out before we cram all these
> > libraries
> > into one git repo.  And again, currently I think distros pull release
> > tarballs
> > and not from any git trees for the rdma provider libraries...
> 
> A git repo can still generate multiple packages/libraries.
>

Agreed.  I'm just asking that we agree on the procedure for how we release
things as part of doing this centralized provider library git repo (assuming we
have consensus that this is the way to move forward).
 
> But, even if you didn't have this, this would just follow the same release
procedure
> that is used by the kernel.

What do you mean?  

Steve. 

--
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: Upcoming libibverbs release

[Hefty, Sean]

> > But, even if you didn't have this, this would just follow the same
> release
> procedure
> > that is used by the kernel.
> 
> What do you mean?

You don't have individual releases for the kernel drivers.  They all just get grouped into the next kernel release.

FWIW, libfabric uses a single repo, and actually builds the providers directly into the library.  That model has been working fine, and the benefits far away the negatives.  I think the concern here is being overstated.
--
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: Upcoming libibverbs release

[Steve Wise]

> > > But, even if you didn't have this, this would just follow the same
> > release
> > procedure
> > > that is used by the kernel.
> >
> > What do you mean?
> 
> You don't have individual releases for the kernel drivers.  They all just get
grouped
> into the next kernel release.
>

Yes, and packaging all this into one libuberverbs is fine, as  long as we can do
point/bug fix releases as needed when individual providers have key fixes they
need pushed into distros...

> FWIW, libfabric uses a single repo, and actually builds the providers directly
into the
> library.  That model has been working fine, and the benefits far away the
negatives.
> I think the concern here is being overstated





--
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: Upcoming libibverbs release

[Steve Wise]

> I think the concern here is being overstated.

I don't think it is being overstated.  It is my concern, and it is valid.
However, after these discussions it has been alleviated to some degree.   It
seems like some of the members of this discussion already have an understanding
of how this would all work out.   But perhaps other provider maintainers need
more details (obviously I did).

Steve.


--
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: Upcoming libibverbs release

[Doug Ledford]

[-- Attachment #1: Type: text/plain, Size: 3355 bytes --]

On 06/29/2016 04:54 PM, Steve Wise wrote:
>> I think the concern here is being overstated.
> 
> I don't think it is being overstated.  It is my concern, and it is valid.
> However, after these discussions it has been alleviated to some degree.   It
> seems like some of the members of this discussion already have an understanding
> of how this would all work out.   But perhaps other provider maintainers need
> more details (obviously I did).

I can tell you how it worked inside Red Hat for me (I no longer handle
the internal builds, others do, so the methodology might have changed a
little bit), but whenever I got ready to do an update for a release, I
had these steps to complete on each package:

1) Download any new source tarball
2) Build/test locally.
3) Build in build system (all official builds must be built through the
build system, which strictly controls how the build root is created for
each build and what packages are installed in that build root to make
sure a package built out of our build system will actually install and
rebuild on a machine of the same release properly).
4) For packages that other package build against, I had to make sure the
new package was put into the build root of the build system so other
packages would get built against the new version and not the old version
of the package.
5) Then I had to make sure the package made its way into an errata so it
would actually make it to the end users eventually.  Because the
packages in the RDMA stack are so incestuously dependent on each other,
I used one errata for all of the RDMA stack packages.  This is in
contrast to the norm at Red Hat which is a separate errata per package.

Mainly because of the dependency nightmare of "have I built libibverbs
and is it in the build root? Yes...OK, which packages can I build
next..", I kept a spreadsheet around to help me keep track of which
packages were on which steps and let me see at a glance if I could even
move to the next step on a given package.  Putting all of the verbs
providers into the same package as libibverbs itself would reduce about
10 of those lines to a single line.  That would certainly make things
easier to track.  And going back to Jason's comments, it wouldn't really
slow down end user's access to the packages since most end users get the
packages from someone like Red Hat and they are all delayed until the
next release anyway.  But, it would be fairly easy to say that we can
have a policy that supra-minor point releases are a foregone conclusion
when a provider library needs an updates.  And just in case that point
isn't clear, let me put it this way:

libuberverbs->master branch - ongoing development
libuberverbs->x.y.0 - Ubervervs major release, major changes or features
present.  When you make this release, you create a verbs-x.y branch.
Primary development continues on master.
libuberverbs->x.y.z - Minor bug fixes to major releases.  Done on
verbs-x.y branch.  Even before bringing in providers, these are
typically used after a major release as bugs settle out.  After adding
providers, it would simply mean that a provider bugfix is an understood
sufficient cause for new minor point release.


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



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 884 bytes --]

RE: Upcoming libibverbs release

[Steve Wise]

> 
> On 06/29/2016 04:54 PM, Steve Wise wrote:
> >> I think the concern here is being overstated.
> >
> > I don't think it is being overstated.  It is my concern, and it is valid.
> > However, after these discussions it has been alleviated to some degree.   It
> > seems like some of the members of this discussion already have an
understanding
> > of how this would all work out.   But perhaps other provider maintainers
need
> > more details (obviously I did).
> 
> I can tell you how it worked inside Red Hat for me (I no longer handle
> the internal builds, others do, so the methodology might have changed a
> little bit), but whenever I got ready to do an update for a release, I
> had these steps to complete on each package:
> 
> 1) Download any new source tarball
> 2) Build/test locally.
> 3) Build in build system (all official builds must be built through the
> build system, which strictly controls how the build root is created for
> each build and what packages are installed in that build root to make
> sure a package built out of our build system will actually install and
> rebuild on a machine of the same release properly).
> 4) For packages that other package build against, I had to make sure the
> new package was put into the build root of the build system so other
> packages would get built against the new version and not the old version
> of the package.
> 5) Then I had to make sure the package made its way into an errata so it
> would actually make it to the end users eventually.  Because the
> packages in the RDMA stack are so incestuously dependent on each other,
> I used one errata for all of the RDMA stack packages.  This is in
> contrast to the norm at Red Hat which is a separate errata per package.
> 
> Mainly because of the dependency nightmare of "have I built libibverbs
> and is it in the build root? Yes...OK, which packages can I build
> next..", I kept a spreadsheet around to help me keep track of which
> packages were on which steps and let me see at a glance if I could even
> move to the next step on a given package.  Putting all of the verbs
> providers into the same package as libibverbs itself would reduce about
> 10 of those lines to a single line.  That would certainly make things
> easier to track.  And going back to Jason's comments, it wouldn't really
> slow down end user's access to the packages since most end users get the
> packages from someone like Red Hat and they are all delayed until the
> next release anyway.  But, it would be fairly easy to say that we can
> have a policy that supra-minor point releases are a foregone conclusion
> when a provider library needs an updates.  And just in case that point
> isn't clear, let me put it this way:
> 
> libuberverbs->master branch - ongoing development
> libuberverbs->x.y.0 - Ubervervs major release, major changes or features
> present.  When you make this release, you create a verbs-x.y branch.
> Primary development continues on master.
> libuberverbs->x.y.z - Minor bug fixes to major releases.  Done on
> verbs-x.y branch.  Even before bringing in providers, these are
> typically used after a major release as bugs settle out.  After adding
> providers, it would simply mean that a provider bugfix is an understood
> sufficient cause for new minor point release.
> 

Thanks Doug.  This last paragraph helps me visualize how it can work with a
libuberverbs process.

Steve.

--
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: Upcoming libibverbs release

[Jason Gunthorpe]

On Wed, Jun 29, 2016 at 02:15:46PM -0500, Steve Wise wrote:

> > I think you start by realizing that nobody uses your git releases
> > anyhow.
> > 
> 
> Not true.  RedHat, for one, does.

I mean no users.

> > Everyone uses either a distro or OFED release and those are already
> > co-ordinated outside your control.
> 
> I don't know who "everyone" is, but at least some of the distros get
> the

everyone = users

> libcxgb3/4 packages from releases I publish and announce on
> linux-rdma.  Chelsio recommends to RedHat exactly which libcxb
> release to pull in for each of their OS releases.  They are not
> necessarily tied to an OFED release either.  Ditto for SUSE,
> although they may still take from OFED packages.

And that doesn't really change, you just recommend a libibverbs
release or libibverbs patch.

> From what I've experienced, the distros don't take patches for
> libraries at all.  They take discreet releases that are published by
> the maintainer of that

Your view might be a bit specialized because a plugin 'library' is a
very special case. Generally the distros will have tight controls on
library updates to guarentee no change to the ABI/API. Often they
won't accept new upstream library versions at all, instead only
backporting necessary fixes.

For instance, if you add the new CQ API support the libcxgb (or any
new API for that matter) then the provider will no longer compile on
the RH released libibverbs and either Doug will have to work on a
backport basis, or you will have to jump through configure hoops to
make it compile.

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: Upcoming libibverbs release

[Doug Ledford]

[-- Attachment #1: Type: text/plain, Size: 2962 bytes --]

On 06/29/2016 03:27 PM, Jason Gunthorpe wrote:
> On Wed, Jun 29, 2016 at 02:15:46PM -0500, Steve Wise wrote:
> 
>>> I think you start by realizing that nobody uses your git releases
>>> anyhow.
>>>
>>
>> Not true.  RedHat, for one, does.
> 
> I mean no users.
> 
>>> Everyone uses either a distro or OFED release and those are already
>>> co-ordinated outside your control.
>>
>> I don't know who "everyone" is, but at least some of the distros get
>> the
> 
> everyone = users

Jason's point, which is correct, is that even though libcxgb3/4 are
separate from libibverbs in their releases, Red Hat (and presumably
other distros), which are the main bodies that take the individual
tarballs, then coordinate between libibverbs and the plugin libraries
before each of our releases.  So your releases are easy for you, and
that gets them directly in the hands of people like Red Hat, but then
they are still coordinated against libibverbs behind the scenes.

>> libcxgb3/4 packages from releases I publish and announce on
>> linux-rdma.  Chelsio recommends to RedHat exactly which libcxb
>> release to pull in for each of their OS releases.  They are not
>> necessarily tied to an OFED release either.  Ditto for SUSE,
>> although they may still take from OFED packages.
> 
> And that doesn't really change, you just recommend a libibverbs
> release or libibverbs patch.

Pretty much.

>> From what I've experienced, the distros don't take patches for
>> libraries at all.  They take discreet releases that are published by
>> the maintainer of that
> 
> Your view might be a bit specialized because a plugin 'library' is a
> very special case. Generally the distros will have tight controls on
> library updates to guarentee no change to the ABI/API. Often they
> won't accept new upstream library versions at all, instead only
> backporting necessary fixes.

Correct, for non-RDMA or kernel related packages.  For the entire RDMA
stack and for specific kernel related packages (like ethtool for
instance), the guidelines are different.

> For instance, if you add the new CQ API support the libcxgb (or any
> new API for that matter) then the provider will no longer compile on
> the RH released libibverbs and either Doug will have to work on a
> backport basis, or you will have to jump through configure hoops to
> make it compile.

That's where the special nature of the RDMA stack comes in to play.
Realistically here, we would batch up the libibverbs update that creates
the API plus all of the driver updates to use the API into one release
and the RPM headers for the various packages would Require: the latest
versions of things and simultaneously Conflict: against old releases of
any packages.  That way we can enforce an all-or-nothing install of
matching verbs packages.


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



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 884 bytes --]

Re: Upcoming libibverbs release

[Yishai Hadas]

On 6/27/2016 9:17 PM, Jason Gunthorpe wrote:
> On Sun, Jun 26, 2016 at 07:54:21PM +0300, Yishai Hadas wrote:
>>> I'm still very much of the opinion that extended CQs should only allow
>>> compatible QP's to be added.
>>
>> Application may create a CQ with large subset of flags that may fit some of
>> its attached QP types. Semantically, it is similar to what we have today, it
>> allows to put different QP types on the same CQ and by thus allow
>> application to have ordered completion events.
>
> This is repeating your argument, I disagree, it is a horrible API
> design, you need to defend this with an actual use case.
>
>> We don't see a good reason to change from current behavior and hurt
>> applications that expect that valid behavior.
>
> Safety for the API consumer.

Disagree, safety should not limit the general usage of an API.
The new API preserves the behavior of the legacy API so that a CQ can be 
attached to more than one QP type.

>>> Add a compatibility layer
>
> Doug and I have both stated we don't want to see so many
> single-provider APIs and churn in libibverbs. This was designed to be
> a generic API that all providers can implement.

The API is generic and can be implemented by any provider, which 
limitation do you see here ?

> The responsibility falls on you to make sure that it works
> universally. Compat is one option.

Compatibility layer can be added in the future, nothing in current 
implementation might block it from happen. No reason to delay a 
libibvers release as of that.

  >>> Exact set of access flags
>>
>> Application can ask for subset of the available bits, in addition, it has an
>> option to ask for all legacy ones (i.e. IBV_WC_STANDARD_FLAGS) for a case
>> that a CQ needs to support all legacy attribute and could be attached to
>> both RC and UD QPs.
>
> Again, this is pretty much nonsense. Asking for data that can't be
> returned is just an insane API design.

See above comment for CQ usage.

In addition, this enum can be used by vendors to sum up the supported WC 
flags instead of repeating the full list of bits. (Look for 
CREATE_CQ_SUPPORTED_WC_FLAGS in libmlx5 code.)

>>> You might want to look at having the caller pass in 'cq' from a member
>>> in ibv_cq_ex. That way the caller can hold the cq in a register
>>> instead of constantly reloading it like this - I'm assuming to_mcq is
>>> not just an container_of??
>>
>> The assumption is *incorrect*, the internal access from ibv_cq_ex to driver
>> CQ is done via offsetof/container_of which is done in compile time, no need
>> to have some change from the clean usage of the API to that mode.
>
> Fine on cq, but the wqe pointer may benifit, and other drivers may
> benifit. It still needs to be studied.

The ibv_wc_read_xxx functions may require some cached context to return 
the correct value. The internal driver CQ can be accessed using 
container_of and if some cached context is needed it can be taken from 
there.

At the moment we don't see a justification to change the clean API that 
uses ibv_cq_ex* as the input.


>> From performance point of view we don't expect a change here, we followed
>> the created assembly code and saw no difference even when it was annotated
>> with 'restrict'.
>
> The annotations help the caller.

We followed also the caller's assembly code but couldn't find a change 
that can justify using 'restrict'.

>> In addition, please note the 'restrict' notation was only became standard as
>> part of C99, in previous compilers it may not work at all or requires
>> different key word to be defined as part of the CONFIG step.
>
> Use __restrict like glibc does.

It may require some logic in the configure script, as pointed above we 
still don't see its benefit.

>> We can go ahead and define the ibv_wc_read_xxx with const qualifier on its
>> input struct ibv_cq_ex *ibcq in some extra incremental patch if makes sense.
>
> Of course it makes sense, you need to fix all of this stuff in all
> the patches you wrote.

We can send a patch for that, let's first agree to ignore the restrict 
if it hasn't approved to be helpful, patch touches same place.
--
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