RDMA/netlink: Hide unimplemented NLDEV commands
PATCH 1/1 RDMA/netlink: Hide unimplemented NLDEV commands
From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
The nldev was implemented by following devlink implementation, including SET/DEL/NEW commands. However these commands were not implemented and hence don't need to be exposed.
--- include/uapi/rdma/rdma_netlink.h | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-)
include/uapi/rdma/rdma_netlink.h +6 -8
@@ -227,16 +227,14 @@ enum rdma_nldev_command {RDMA_NLDEV_CMD_UNSPEC ,RDMA_NLDEV_CMD_GET ,/* can dump */ -RDMA_NLDEV_CMD_SET , -RDMA_NLDEV_CMD_NEW , -RDMA_NLDEV_CMD_DEL , -RDMA_NLDEV_CMD_PORT_GET ,/* can dump */ -RDMA_NLDEV_CMD_PORT_SET , -RDMA_NLDEV_CMD_PORT_NEW , -RDMA_NLDEV_CMD_PORT_DEL , +/* 2 - 4 are free to use */ -RDMA_NLDEV_CMD_RES_GET ,/* can dump */ +RDMA_NLDEV_CMD_PORT_GET =5 ,/* can dump */ + +/* 6 - 8 are free to use */ + +RDMA_NLDEV_CMD_RES_GET =9 ,/* can dump */ RDMA_NLDEV_CMD_RES_QP_GET, /* can dump */
--
2.16.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): 20180130150716.16372-1-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
Quoted 23 lines · Tue, Jan 30, 2018 at 04:07:16PM CET, [email protected]…
From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
The nldev was implemented by following devlink implementation, including SET/DEL/NEW commands. However these commands were not implemented and hence don't need to be exposed.
---
include/uapi/rdma/rdma_netlink.h| 14 ++++++--------1 file changed, 6 insertions(+), 8 deletions(-)include/uapi/rdma/rdma_netlink.h +0
-3@@ -227,16 +227,14 @@ enum rdma_nldev_command {RDMA_NLDEV_CMD_UNSPEC , RDMA_NLDEV_CMD_GET, /* can dump */ -RDMA_NLDEV_CMD_SET , -RDMA_NLDEV_CMD_NEW , - RDMA_NLDEV_CMD_DEL,
include/uapi/rdma/rdma_netlink.h enum rdma_nldev_command { · line 235
Quoted 10 lines
include/uapi/rdma/rdma_netlink.h +4
-5-RDMA_NLDEV_CMD_PORT_GET ,/* can dump */ -RDMA_NLDEV_CMD_PORT_SET , -RDMA_NLDEV_CMD_PORT_NEW , -RDMA_NLDEV_CMD_PORT_DEL , +/* 2 - 4 are free to use */ - RDMA_NLDEV_CMD_RES_GET, /* can dump */ + RDMA_NLDEV_CMD_PORT_GET = 5, /* can dump */ + +/* 6 - 8 are free to use */
I don't see reason for the wholes. Also, Don't you use *_NEW for replies to *_GET, as it is common for Netlink?
But anyway, I would rather have them in, or don't have them in but don't leave holes for them.
Quoted 8 lines
include/uapi/rdma/rdma_netlink.h +2
-1+ +RDMA_NLDEV_CMD_RES_GET =9 ,/* can dump */ RDMA_NLDEV_CMD_RES_QP_GET, /* can dump */ --2.16.1
Quoted 4 lines
--
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
--
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
Quoted 28 lines · Jiri Pirko
Quoted 27 lines · Tue, Jan 30, 2018 at 04:07:16PM CET, [email protected]…
From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
The nldev was implemented by following devlink implementation, including SET/DEL/NEW commands. However these commands were not implemented and hence don't need to be exposed.
include/uapi/rdma/rdma_netlink.h| 14 ++++++--------1 file changed, 6 insertions(+), 8 deletions(-)include/uapi/rdma/rdma_netlink.h +2
-7+++ b/include/uapi/rdma/rdma_netlink.h@@ -227,16 +227,14 @@ enum rdma_nldev_command {RDMA_NLDEV_CMD_UNSPEC , RDMA_NLDEV_CMD_GET, /* can dump */ -RDMA_NLDEV_CMD_SET , -RDMA_NLDEV_CMD_NEW , -RDMA_NLDEV_CMD_DEL , -RDMA_NLDEV_CMD_PORT_GET ,/* can dump */ -RDMA_NLDEV_CMD_PORT_SET , -RDMA_NLDEV_CMD_PORT_NEW , - RDMA_NLDEV_CMD_PORT_DEL, +/* 2 - 4 are free to use */
Quoted 6 lines
include/uapi/rdma/rdma_netlink.h enum rdma_nldev_command { · line 235
Quoted 4 lines
include/uapi/rdma/rdma_netlink.h +3
-1- RDMA_NLDEV_CMD_RES_GET, /* can dump */ + RDMA_NLDEV_CMD_PORT_GET = 5, /* can dump */ + +/* 6 - 8 are free to use */ I don't see reason for the wholes.
We are stuck with the holes, it is fixed uABI at this point, can't change the constants we actually implemented.
Quoted 2 lines
Also, Don't you use
*_NEWfor replies to*_GET, as it is common for Netlink?
It appears not..
IMHO that convention makes the most sense when there is a GET/NEW pair with the idea the NEW is formed to be echoed back to the kernel - these case are all GET only..
Jason
--
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): 20180130160337.GD17053-uk2M96/[email protected]
Quoted 36 lines · Tue, Jan 30, 2018 at 05:03:37PM CET, jgg-uk2M96/[email protected] wrote:
Quoted 28 lines · Jiri Pirko
Quoted 27 lines · Tue, Jan 30, 2018 at 04:07:16PM CET, [email protected]…
From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
The nldev was implemented by following devlink implementation, including SET/DEL/NEW commands. However these commands were not implemented and hence don't need to be exposed.
include/uapi/rdma/rdma_netlink.h| 14 ++++++--------1 file changed, 6 insertions(+), 8 deletions(-)include/uapi/rdma/rdma_netlink.h +2
-7+++ b/include/uapi/rdma/rdma_netlink.h@@ -227,16 +227,14 @@ enum rdma_nldev_command {RDMA_NLDEV_CMD_UNSPEC , RDMA_NLDEV_CMD_GET, /* can dump */ -RDMA_NLDEV_CMD_SET , -RDMA_NLDEV_CMD_NEW , -RDMA_NLDEV_CMD_DEL , -RDMA_NLDEV_CMD_PORT_GET ,/* can dump */ -RDMA_NLDEV_CMD_PORT_SET , -RDMA_NLDEV_CMD_PORT_NEW , - RDMA_NLDEV_CMD_PORT_DEL, +/* 2 - 4 are free to use */ Quoted 6 lines
include/uapi/rdma/rdma_netlink.h enum rdma_nldev_command { · line 235
Quoted 4 lines
include/uapi/rdma/rdma_netlink.h +3
-1- RDMA_NLDEV_CMD_RES_GET, /* can dump */ + RDMA_NLDEV_CMD_PORT_GET = 5, /* can dump */ + +/* 6 - 8 are free to use */ I don't see reason for the wholes.
Quoted 2 lines
We are stuck with the holes, it is fixed uABI at this point, can't change the constants we actually implemented.
If it is realeased already, we can't remove the defines either.
Quoted 11 lines
Quoted 3 lines
Also, Don't you use
*_NEWfor replies to*_GET, as it is common for Netlink?It appears not..
IMHO that convention makes the most sense when there is a GET/NEW pair with the idea the NEW is formed to be echoed back to the kernel - these case are all GET only..
Jason
--
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
Quoted 37 lines · Jiri Pirko
Quoted 36 lines · Tue, Jan 30, 2018 at 05:03:37PM CET, jgg-uk2M96/[email protected] wrote:
Quoted 34 lines · Jiri Pirko
Quoted 21 lines · Tue, Jan 30, 2018 at 04:07:16PM CET, [email protected]…
From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
The nldev was implemented by following devlink implementation, including SET/DEL/NEW commands. However these commands were not implemented and hence don't need to be exposed.
include/uapi/rdma/rdma_netlink.h| 14 ++++++--------1 file changed, 6 insertions(+), 8 deletions(-)include/uapi/rdma/rdma_netlink.h +1
-3+++ b/include/uapi/rdma/rdma_netlink.h@@ -227,16 +227,14 @@ enum rdma_nldev_command {RDMA_NLDEV_CMD_UNSPEC , RDMA_NLDEV_CMD_GET, /* can dump */ -RDMA_NLDEV_CMD_SET , -RDMA_NLDEV_CMD_NEW , - RDMA_NLDEV_CMD_DEL,include/uapi/rdma/rdma_netlink.h enum rdma_nldev_command { · line 235
Quoted 10 lines
include/uapi/rdma/rdma_netlink.h +4
-5-RDMA_NLDEV_CMD_PORT_GET ,/* can dump */ -RDMA_NLDEV_CMD_PORT_SET , -RDMA_NLDEV_CMD_PORT_NEW , -RDMA_NLDEV_CMD_PORT_DEL , +/* 2 - 4 are free to use */ - RDMA_NLDEV_CMD_RES_GET, /* can dump */ + RDMA_NLDEV_CMD_PORT_GET = 5, /* can dump */ + +/* 6 - 8 are free to use */ I don't see reason for the wholes.
Quoted 4 lines
Quoted 2 lines
We are stuck with the holes, it is fixed uABI at this point, can't change the constants we actually implemented.
If it is realeased already, we can't remove the defines either.
Why? There is exactly one consumer - rdmatool and it doesn't use removed constants.
Quoted 17 lines
Quoted 6 lines
Quoted 5 lines
Also, Don't you use
*_NEWfor replies to*_GET, as it is common for Netlink?Quoted 7 lines
It appears not..
IMHO that convention makes the most sense when there is a GET/NEW pair with the idea the NEW is formed to be echoed back to the kernel - these case are all GET only..
Jason
--
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
Quoted 37 lines · Jiri Pirko
Quoted 36 lines · Tue, Jan 30, 2018 at 05:03:37PM CET, jgg-uk2M96/[email protected] wrote:
Quoted 34 lines · Jiri Pirko
Quoted 21 lines · Tue, Jan 30, 2018 at 04:07:16PM CET, [email protected]…
From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
The nldev was implemented by following devlink implementation, including SET/DEL/NEW commands. However these commands were not implemented and hence don't need to be exposed.
include/uapi/rdma/rdma_netlink.h| 14 ++++++--------1 file changed, 6 insertions(+), 8 deletions(-)include/uapi/rdma/rdma_netlink.h +1
-3+++ b/include/uapi/rdma/rdma_netlink.h@@ -227,16 +227,14 @@ enum rdma_nldev_command {RDMA_NLDEV_CMD_UNSPEC , RDMA_NLDEV_CMD_GET, /* can dump */ -RDMA_NLDEV_CMD_SET , -RDMA_NLDEV_CMD_NEW , - RDMA_NLDEV_CMD_DEL,include/uapi/rdma/rdma_netlink.h enum rdma_nldev_command { · line 235
Quoted 10 lines
include/uapi/rdma/rdma_netlink.h +4
-5-RDMA_NLDEV_CMD_PORT_GET ,/* can dump */ -RDMA_NLDEV_CMD_PORT_SET , -RDMA_NLDEV_CMD_PORT_NEW , -RDMA_NLDEV_CMD_PORT_DEL , +/* 2 - 4 are free to use */ - RDMA_NLDEV_CMD_RES_GET, /* can dump */ + RDMA_NLDEV_CMD_PORT_GET = 5, /* can dump */ + +/* 6 - 8 are free to use */ I don't see reason for the wholes.
Quoted 4 lines
Quoted 2 lines
We are stuck with the holes, it is fixed uABI at this point, can't change the constants we actually implemented.
If it is realeased already, we can't remove the defines either.
We have been less absolute on the uapi headers causing compilation breaks.
Using these headers is so hard, eg iproute and rdma-core take them verbatim from the kernel and bundles them inside. So as a practical matter, it is OK to sometimes cause compilation issues.
In this case there is no correct use of the constants since the kernel never implemented them, any user is busted in some way and should be reviewed.
Even glibc causes compilation breaks in new versions..
Jason
--
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
Quoted 39 lines · Jason Gunthorpe
Quoted 28 lines · Jiri Pirko
Quoted 27 lines · Tue, Jan 30, 2018 at 04:07:16PM CET, [email protected]…
From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
The nldev was implemented by following devlink implementation, including SET/DEL/NEW commands. However these commands were not implemented and hence don't need to be exposed.
include/uapi/rdma/rdma_netlink.h| 14 ++++++--------1 file changed, 6 insertions(+), 8 deletions(-)include/uapi/rdma/rdma_netlink.h +2
-7+++ b/include/uapi/rdma/rdma_netlink.h@@ -227,16 +227,14 @@ enum rdma_nldev_command {RDMA_NLDEV_CMD_UNSPEC , RDMA_NLDEV_CMD_GET, /* can dump */ -RDMA_NLDEV_CMD_SET , -RDMA_NLDEV_CMD_NEW , -RDMA_NLDEV_CMD_DEL , -RDMA_NLDEV_CMD_PORT_GET ,/* can dump */ -RDMA_NLDEV_CMD_PORT_SET , -RDMA_NLDEV_CMD_PORT_NEW , - RDMA_NLDEV_CMD_PORT_DEL, +/* 2 - 4 are free to use */ Quoted 6 lines
include/uapi/rdma/rdma_netlink.h enum rdma_nldev_command { · line 235
Quoted 4 lines
include/uapi/rdma/rdma_netlink.h +3
-1- RDMA_NLDEV_CMD_RES_GET, /* can dump */ + RDMA_NLDEV_CMD_PORT_GET = 5, /* can dump */ + +/* 6 - 8 are free to use */ I don't see reason for the wholes.
We are stuck with the holes, it is fixed uABI at this point, can't change the constants we actually implemented.
Quoted 8 lines
Quoted 2 lines
Also, Don't you use
*_NEWfor replies to*_GET, as it is common for Netlink?It appears not..
IMHO that convention makes the most sense when there is a GET/NEW pair with the idea the NEW is formed to be echoed back to the kernel - these case are all GET only..
Back then, I didn't find any reason to return _NEW, and I don't see it now either.
There is NLM_F_ECHO flag for echo and sequence number to distinguish between messages.
Quoted 2 lines
Jason
Quoted 9 lines · Leon Romanovsky
From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
The nldev was implemented by following devlink implementation, including SET/DEL/NEW commands. However these commands were not implemented and hence don't need to be exposed.
include/uapi/rdma/rdma_netlink.h | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-)
Doug applied this to for-next
Jason
--
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