Re: [PATCH rdma-rc v1] RDMA: Remove kernel private defines and reference to header from UAPI

[Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]

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

On Wed, Feb 08, 2017 at 05:15:49PM +0200, Matan Barak (External) wrote:
> On 08/02/2017 17:04, Leon Romanovsky wrote:
> > Remove references to private kernel header and defines from exported
> > ib_user_verb.h file.
> >
> > The code snippet below is used to reproduce the issue:
> >
> >  #include <stdio.h>
> >  #include <rdma/ib_user_verb.h>
> >
> >  int main(void)
> >  {
> > 	printf("IB_USER_VERBS_ABI_VERSION = %d\n", IB_USER_VERBS_ABI_VERSION);
> > 	return 0;
> >  }
> >
> > It fails during compilation phase with an error:
> > ➜  /tmp gcc main.c
> > main.c:2:31: fatal error: rdma/ib_user_verb.h: No such file or directory
> >  #include <rdma/ib_user_verb.h>
> >                                ^
> > compilation terminated.
> >
> > Fixes: 189aba99e700 ("IB/uverbs: Extend modify_qp and support packet pacing")
> > CC: Bodong Wang <bodong-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > CC: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > CC: Christoph Hellwig <hch-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
> > Tested-by: Slava Shwartsman <slavash-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> > ---
> > Changelog v0 -> v1
> >  * Document the origin of hard-coded values.
> > ---
> >  include/uapi/rdma/ib_user_verbs.h | 11 ++++++++---
> >  1 file changed, 8 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/uapi/rdma/ib_user_verbs.h b/include/uapi/rdma/ib_user_verbs.h
> > index dfdfe4e92d31..f4f87cff6dc6 100644
> > --- a/include/uapi/rdma/ib_user_verbs.h
> > +++ b/include/uapi/rdma/ib_user_verbs.h
> > @@ -37,7 +37,6 @@
> >  #define IB_USER_VERBS_H
> >
> >  #include <linux/types.h>
> > -#include <rdma/ib_verbs.h>
> >
> >  /*
> >   * Increment this value if any changes that break userspace ABI
> > @@ -548,11 +547,17 @@ enum {
> >  };
> >
> >  enum {
> > -	IB_USER_LEGACY_LAST_QP_ATTR_MASK = IB_QP_DEST_QPN
> > +	/*
> > +	 * This value is equal to IB_QP_DEST_QPN.
> > +	 */
> > +	IB_USER_LEGACY_LAST_QP_ATTR_MASK = 1ULL << 20,
> >  };
> >
> >  enum {
> > -	IB_USER_LAST_QP_ATTR_MASK = IB_QP_RATE_LIMIT
> > +	/*
> > +	 * This value is equal to IB_QP_RATE_LIMIT.
> > +	 */
> > +	IB_USER_LAST_QP_ATTR_MASK = 1ULL << 25,
> >  };
> >
> >  struct ib_uverbs_ex_create_qp {
> > --
> > 2.11.1
> >
>
> I tried to write this earlier, but for some reason my mailer didn't really
> send it.
> IMHO, since these enum values are actually passed between user-space and
> kernel (attr_mask), it's acceptable to expose all enum values.
> Otherwise, user-space should define all these symbols by by itself (as done
> today), so why bother introduce only this explicit symbol?

Mainly because it is -rc7 fix and it is MUST be applied as soon
as possible, currently IB stack is broken.

> --
> 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]