| Message ID | 20170208150409.29537-1-leon@kernel.org (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
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@mellanox.com> > CC: Matan Barak <matanb@mellanox.com> > CC: Christoph Hellwig <hch@infradead.org> > Tested-by: Slava Shwartsman <slavash@mellanox.com> > Signed-off-by: Leon Romanovsky <leon@kernel.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? -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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@mellanox.com> > > CC: Matan Barak <matanb@mellanox.com> > > CC: Christoph Hellwig <hch@infradead.org> > > Tested-by: Slava Shwartsman <slavash@mellanox.com> > > Signed-off-by: Leon Romanovsky <leon@kernel.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@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 2017-02-08 at 19:25 +0200, Leon Romanovsky wrote: > > 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. Indeed, which is why I took the V1 of the patch. We can fix it up nicer in -next if people prefer.
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 {
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@mellanox.com> CC: Matan Barak <matanb@mellanox.com> CC: Christoph Hellwig <hch@infradead.org> Tested-by: Slava Shwartsman <slavash@mellanox.com> Signed-off-by: Leon Romanovsky <leon@kernel.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(-) -- 2.11.1 -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html