Skip to contentSkip to conversation

RDMA: Remove kernel private defines and reference to header from UAPI

PATCH In review Leon Romanovsky 4 participants 7 messages Feb 7, 2017 – Feb 8, 2017 4 messages not in the archive InfiniBand

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 RDMA: Remove kernel private defines and reference to header from UAPI

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/[email protected]>
CC Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/[email protected]>
---
 include/uapi/rdma/ib_user_verbs.h | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
include/uapi/rdma/ib_user_verbs.h +2 -3
index dfdfe4e92d31..c56f525b041d 100644
@@ -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,11 @@ enum {
 };
 
 enum {
-	IB_USER_LEGACY_LAST_QP_ATTR_MASK = IB_QP_DEST_QPN
+	IB_USER_LEGACY_LAST_QP_ATTR_MASK = 1ULL << 20,
 };
 
 enum {
-	IB_USER_LAST_QP_ATTR_MASK = IB_QP_RATE_LIMIT
+	IB_USER_LAST_QP_ATTR_MASK = 1ULL << 25,
 };
 
 struct ib_uverbs_ex_create_qp {

--
2.11.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 20170207174600.27218-1-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org

Tested-by Slava Shwartsman
#2 lore raw

Works for me.

Quoted 63 lines · Leon Romanovsky

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/[email protected]>
CC Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/[email protected]>
---
 include/uapi/rdma/ib_user_verbs.h | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
include/uapi/rdma/ib_user_verbs.h +2 -4
index dfdfe4e92d31..c56f525b041d 100644
@@ -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,11 @@ enum {
 };
 
 enum {
-       IB_USER_LEGACY_LAST_QP_ATTR_MASK = IB_QP_DEST_QPN
+       IB_USER_LEGACY_LAST_QP_ATTR_MASK = 1ULL << 20,
 };
 
 enum {
-       IB_USER_LAST_QP_ATTR_MASK = IB_QP_RATE_LIMIT
+       IB_USER_LAST_QP_ATTR_MASK = 1ULL << 25,
 };
 
 struct ib_uverbs_ex_create_qp {
--

2.11.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Signature · 7 lines
Thank you in advance,

Slava Shwartsman
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#3 lore raw
In reply to a message not in the archive
Quoted 6 lines

#include <linux/types.h>

include/uapi/rdma/ib_user_verbs.h +0 -1
-#include <rdma/ib_verbs.h>
 
 /*
  * Increment this value if any changes that break userspace ABI

include/uapi/rdma/ib_user_verbs.h enum { · line 554

Quoted 10 lines
include/uapi/rdma/ib_user_verbs.h +2 -2
@@ -548,11 +547,11 @@ enum {
 };
 
 enum {
-	IB_USER_LEGACY_LAST_QP_ATTR_MASK = IB_QP_DEST_QPN
+	IB_USER_LEGACY_LAST_QP_ATTR_MASK = 1ULL << 20,
 };
 
 enum {
-	IB_USER_LAST_QP_ATTR_MASK = IB_QP_RATE_LIMIT
+	IB_USER_LAST_QP_ATTR_MASK = 1ULL << 25,

I disagree with the upendcoding. These constant should be moved to the user verbs header instead.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): [email protected]

#5 lore raw
Quoted 13 lines · Christoph Hellwig
Quoted 13 lines

#include <linux/types.h>

include/uapi/rdma/ib_user_verbs.h +1 -2
-#include <rdma/ib_verbs.h>
 
 /*
  * Increment this value if any changes that break userspace ABI
@@ -548,11 +547,11 @@ enum {
 };
 
 enum {
-	IB_USER_LEGACY_LAST_QP_ATTR_MASK = IB_QP_DEST_QPN
+	IB_USER_LEGACY_LAST_QP_ATTR_MASK = 1ULL << 20,
 };
 
Quoted 6 lines

include/uapi/rdma/ib_user_verbs.h enum { · line 554

Quoted 3 lines
include/uapi/rdma/ib_user_verbs.h +1 -1
 enum {
-	IB_USER_LAST_QP_ATTR_MASK = IB_QP_RATE_LIMIT
+	IB_USER_LAST_QP_ATTR_MASK = 1ULL << 25,

I disagree with the upendcoding. These constant should be moved to the user verbs header instead.

These constants are part of much larger enum ib_qp_attr_mask. IMHO copy two values from that enum isn't good, but copy whole enum (mostly not needed for the users) is bad either. So I decided to open code it as a fix for -rc7.

Thanks

A message that is not in the archive (sent off-list or not yet received): 20170208100043.GC6005-U/[email protected]

#7 lore raw
Quoted 4 lines · Leon Romanovsky

These constants are part of much larger enum ib_qp_attr_mask. IMHO copy two values from that enum isn't good, but copy whole enum (mostly not needed for the users) is bad either. So I decided to open code it as a fix for -rc7.

At least add comments mentioning the proper symbolic values then. --
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): [email protected]

#9 lore raw
Quoted 7 lines · Christoph Hellwig
Quoted 4 lines · Leon Romanovsky

These constants are part of much larger enum ib_qp_attr_mask. IMHO copy two values from that enum isn't good, but copy whole enum (mostly not needed for the users) is bad either. So I decided to open code it as a fix for -rc7.

At least add comments mentioning the proper symbolic values then.

Sure, I'll send an update.

Thanks

#10 lore raw
In reply to a message not in the archive
Quoted 21 lines · Leon Romanovsky
Quoted 13 lines · Christoph Hellwig
Quoted 13 lines

#include <linux/types.h>

include/uapi/rdma/ib_user_verbs.h +1 -2
-#include <rdma/ib_verbs.h>
 
 /*
  * Increment this value if any changes that break userspace ABI
@@ -548,11 +547,11 @@ enum {
 };
 
 enum {
-	IB_USER_LEGACY_LAST_QP_ATTR_MASK = IB_QP_DEST_QPN
+	IB_USER_LEGACY_LAST_QP_ATTR_MASK = 1ULL << 20,
 };
 
Quoted 6 lines

include/uapi/rdma/ib_user_verbs.h enum { · line 554

Quoted 3 lines
include/uapi/rdma/ib_user_verbs.h +1 -1
 enum {
-	IB_USER_LAST_QP_ATTR_MASK = IB_QP_RATE_LIMIT
+	IB_USER_LAST_QP_ATTR_MASK = 1ULL << 25,

I disagree with the upendcoding. These constant should be moved to the user verbs header instead.

Quoted 7 lines

These constants are part of much larger enum ib_qp_attr_mask. IMHO copy two values from that enum isn't good, but copy whole enum (mostly not needed for the users) is bad either. So I decided to open code it as a fix for -rc7.

Thanks

IMHO, since these enum values are actually passed between user-space to kernel (attr_mask), it's acceptable to expose all enum values. Otherwise, the user-space should define all these symbols by by itself, 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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html