RDMA netlink refactoring and RDMAtool code
Subsystems (9)
Latest code drop with fixed LMC name (LID mask control).
----------------------------------------------------------------
The following changes since commit 913cc67159bc85a96d94df301ca39c1b2c540dca:
IB/hfi1: Always perform offline transition (2017-07-31 15:18:38 -0400)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/leon/linux-rdma.git tags/rdma-next-2017-08-01
for you to fetch changes up to 76bed394b018e89cc0a7d69a2c724655a1506c2c:
RDMA/netlink: Export node_type (2017-08-01 14:22:50 +0300)
----------------------------------------------------------------
RDMA netlink refactoring and RDMAtool code
----------------------------------------------------------------
Leon Romanovsky (33):
Revert "IB/core: Add flow control to the portmapper netlink calls" RDMA/netlink: Remove netlink clients infrastructure RDMA/netlink: Remove redundant owner option for netlink callbacks RDMA/netlink: Avoid double pass for RDMA netlink messages RDMA/iwcm: Remove useless check of nelink client validity RDMA/iwcm: Remove extra EXPORT_SYMBOLS
RDMA/netlink: Add flag to consolidate common handing RDMA/netlink: Simplify the put_msg and put_attr RDMA/netlink: Rename and remove redundant parameter from ibnl_unicast RDMA/netlink: Rename and remove redundant parameter from ibnl_multicast RDMA/netlink: Simplify and rename ibnl_chk_listeners RDMA/netlink: Rename netlink callback struct
RDMA/core: Add iterator over ib_devices
RDMA/core: Add and expose static device index
RDMA/netlink: Add and implement doit netlink callback RDMA/netlink: Reduce indirection access to cb_table RDMA/netlink: Convert LS to doit callback
RDMA/netlink: Update copyright
RDMA/netlink: Add netlink device definitions to UAPI RDMA/netlink: Add nldev initialization flows
RDMA/netlink: Implement nldev device dumpit calback RDMA/netlink: Add nldev device doit implementation RDMA/netlink: Add nldev port dumpit implementation RDMA/netlink: Implement nldev port doit callback RDMA/netlink: Expose device and port capability masks
RDMA: Simplify get firmware interface
RDMA/netlink: Export FW version
RDMA/netlink: Export node_guid and sys_image_guid RDMA/netlink: Advertise IB subnet prefix
RDMA/netink: Export lids and sm_lids
RDMA/netlink: Export LID mask control (LMC)
RDMA/netlink: Provide port state and physical link state RDMA/netlink: Export node_type
drivers/infiniband/core/Makefile | 3 +- drivers/infiniband/core/addr.c | 12 +- drivers/infiniband/core/cma.c | 11 +- drivers/infiniband/core/core_priv.h | 26 ++- drivers/infiniband/core/device.c | 119 +++++++--- drivers/infiniband/core/iwcm.c | 12 +- drivers/infiniband/core/iwpm_msg.c | 20 +- drivers/infiniband/core/iwpm_util.c | 15 +- drivers/infiniband/core/netlink.c | 310 ++++++++++++------------ drivers/infiniband/core/nldev.c | 312 +++++++++++++++++++++++++ drivers/infiniband/core/sa_query.c | 18 +- drivers/infiniband/core/sysfs.c | 4 +- drivers/infiniband/hw/cxgb3/iwch_provider.c | 5 +- drivers/infiniband/hw/cxgb4/provider.c | 5 +- drivers/infiniband/hw/hfi1/verbs.c | 5 +- drivers/infiniband/hw/i40iw/i40iw_verbs.c | 7 +- drivers/infiniband/hw/mlx4/main.c | 5 +- drivers/infiniband/hw/mlx5/main.c | 8 +- drivers/infiniband/hw/mthca/mthca_provider.c | 5 +- drivers/infiniband/hw/nes/nes_verbs.c | 5 +- drivers/infiniband/hw/ocrdma/ocrdma_main.c | 5 +- drivers/infiniband/hw/qedr/main.c | 5 +- drivers/infiniband/hw/usnic/usnic_ib_main.c | 6 +- drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c | 5 +- drivers/infiniband/ulp/ipoib/ipoib_ethtool.c | 3 +- include/rdma/ib_verbs.h | 8 +- include/rdma/rdma_netlink.h | 38 +-- include/uapi/rdma/rdma_netlink.h | 84 ++++++- 28 files changed, 743 insertions(+), 318 deletions(-) create mode 100644 drivers/infiniband/core/nldev.c
--
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): 20170801120536.540-1-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
PATCH 1/33 Revert "IB/core: Add flow control to the portmapper netlink calls"
From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
The commit cea05eadded0 ("IB/core: Add flow control to the portmapper netlink calls") changed netlink to be blocked for all RDMA clients. This workaround worked perfectly for portmapper, but is not correct for the whole NETLINK_RDMA family.
The request/response should always be blocking and asynchronous notification should always be non-blocking.
It is library and user-space application to chose how to handle recvmgs, as an example see nl_recvmsgs() and nl_socket_set_nonblocking() calls of libnl library.
Send timeout is not needed too and can be configured with SO_SNDTIMEO socket option.
This reverts commit cea05eadded0d4eb59f7be6e1f1560eb6bfde2bf.
--- drivers/infiniband/core/netlink.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)
drivers/infiniband/core/netlink.c +1 -5
@@ -230,10 +230,7 @@ static void ibnl_rcv(struct sk_buff *skb)int ibnl_unicast (struct sk_buff *skb,struct nlmsghdr *nlh,__u32 pid) { -int err; - - err =netlink_unicast (nls, skb, pid,0 ); -return (err <0 ) ? err :0 ; +return nlmsg_unicast (nls, skb, pid); }EXPORT_SYMBOL (ibnl_unicast);
@@ -256,7 +253,6 @@ int __init ibnl_init(void)return -ENOMEM ; } - nls->sk_sndtimeo =10 *HZ ;return 0 ; }
Signature · 6 lines
2.13.3 -- 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): 20170801120536.540-2-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"
Quoted 6 lines · Leon Romanovsky
From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
The commit
cea05eadded0("IB/core: Add flow control to the portmapper netlink calls") changed netlink to be blocked for all RDMA clients. This workaround worked perfectly for portmapper, but is not correct for the wholeNETLINK_RDMAfamily.
Leon,
I've already told you I'm opposed to the revert. There is a patch that will work with your usage of ibnl_unicast() but you chose to abandon that discussion on June 29, 2017 (RDMA/core: Add wait/retry version of ibnl_unicast). Either you step up with good sound technical evidence to convince me that patch won't work for you or you stop trying to break existing functionality with this revert.
Chien
--
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): 20170801133832.GA11812-TZeIlv3TuzOfrEmaQUPKxl95YUYmaKo1UNDiOz3kqAs@public.gmane.org
Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"
Quoted 8 lines · Chien Tin Tung
Quoted 6 lines · Leon Romanovsky
From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
The commit
cea05eadded0("IB/core: Add flow control to the portmapper netlink calls") changed netlink to be blocked for all RDMA clients. This workaround worked perfectly for portmapper, but is not correct for the wholeNETLINK_RDMAfamily.
Quoted 7 lines
Leon,
I've already told you I'm opposed to the revert. There is a patch that will work with your usage of
ibnl_unicast()but you chose to abandon that discussion on June 29, 2017 (RDMA/core: Add wait/retry version ofibnl_unicast). Either you step up with good sound technical evidence to convince me that patch won't work for you or you stop trying to break existing functionality with this revert.
Chien,
You never explained us what exactly your original patch fixed and why it should be fixed in kernel and not in user space. I saw that our discussion wasn't useful and brought bad blood instead of good will, so I stepped out in waited for RDMA maintainer (Doug) step in.
I never heard anything from Doug on the matter, so proceeded and resubmitted it.
Thanks
Quoted 2 lines
Chien
A message that is not in the archive (sent off-list or not yet received): 20170801141023.GM13672-U/[email protected]
Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"
Quoted 17 lines · Leon Romanovsky
Quoted 8 lines · Chien Tin Tung
Quoted 6 lines · Leon Romanovsky
From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
The commit
cea05eadded0("IB/core: Add flow control to the portmapper netlink calls") changed netlink to be blocked for all RDMA clients. This workaround worked perfectly for portmapper, but is not correct for the wholeNETLINK_RDMAfamily.Quoted 7 lines
Leon,
I've already told you I'm opposed to the revert. There is a patch that will work with your usage of
ibnl_unicast()but you chose to abandon that discussion on June 29, 2017 (RDMA/core: Add wait/retry version ofibnl_unicast). Either you step up with good sound technical evidence to convince me that patch won't work for you or you stop trying to break existing functionality with this revert.
Quoted 6 lines
Chien,
You never explained us what exactly your original patch fixed and why it should be fixed in kernel and not in user space. I saw that our discussion wasn't useful and brought bad blood instead of good will, so I stepped out in waited for RDMA maintainer (Doug) step in.
Why do I need to explain the original patch? It was accepted many kernels ago. Your questions on the original were based on false assumptions and facts which I've proven over and over. You are right in that I do not want to revisit those either. There is a patch that can solve the problem you are facing but yet you insist on the revert. This is very puzzling to me. You simply refuse to move this forward. That is your choice but revert is a no go for me.
Quoted 2 lines
I never heard anything from Doug on the matter, so proceeded and resubmitted it.
Fine. Your choice to resubmit and my choice to say no to the revert.
Chien
--
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): 20170801141842.GA1808-TZeIlv3TuzOfrEmaQUPKxl95YUYmaKo1UNDiOz3kqAs@public.gmane.org
Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"
Quoted 7 lines · Chien Tin Tung
Why do I need to explain the original patch? It was accepted many kernels ago. Your questions on the original were based on false assumptions and facts which I've proven over and over. You are right in that I do not want to revisit those either. There is a patch that can solve the problem you are facing but yet you insist on the revert. This is very puzzling to me. You simply refuse to move this forward. That is your choice but revert is a no go for me.
Ok this is pretty confusing to someone not involved in the prior discussions. Could both of your stop attacking each other and start talking about the technical issues?
--
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
Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"
Quoted 10 lines · Christopher Lameter
Quoted 7 lines · Chien Tin Tung
Why do I need to explain the original patch? It was accepted many kernels ago. Your questions on the original were based on false assumptions and facts which I've proven over and over. You are right in that I do not want to revisit those either. There is a patch that can solve the problem you are facing but yet you insist on the revert. This is very puzzling to me. You simply refuse to move this forward. That is your choice but revert is a no go for me.
Quoted 4 lines
Ok this is pretty confusing to someone not involved in the prior discussions. Could both of your stop attacking each other and start talking about the technical issues?
Christopher,
I'm reverting the patch which made two things to RDMA netlink - It made the netlink stack blocking, which shouldn't. And reinvented the wheel with setting timeout on socket, instead of using proper and already exposed API.
I tried to explain it in commit message.
Chien sent patch which supposed to replace my revert, but both of his and my patch wasn't accepted without any reason, so I proceed.
Thanks
Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"
In reply to Christopher LameterQuoted 10 lines · Christopher Lameter
Quoted 7 lines · Chien Tin Tung
Why do I need to explain the original patch? It was accepted many kernels ago. Your questions on the original were based on false assumptions and facts which I've proven over and over. You are right in that I do not want to revisit those either. There is a patch that can solve the problem you are facing but yet you insist on the revert. This is very puzzling to me. You simply refuse to move this forward. That is your choice but revert is a no go for me.
Quoted 3 lines
Ok this is pretty confusing to someone not involved in the prior discussions. Could both of your stop attacking each other and start talking about the technical issues?
There's been multiple threads over this revert. This one is the latest Leon walked away from: https://patchwork.kernel.org/patch/9814367/
I'm simply asking for proof/evidence/facts to back up Leon's claims. Show me the code/stack trace/whatever and I will be happy to admit wrong publicly on the list and move on. I do have better things to do than to NACK a revert.
Chien
--
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): 20170801151511.GA13376-TZeIlv3TuzOfrEmaQUPKxl95YUYmaKo1UNDiOz3kqAs@public.gmane.org
Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"
Quoted 15 lines · Chien Tin Tung
Quoted 10 lines · Christopher Lameter
Quoted 7 lines · Chien Tin Tung
Why do I need to explain the original patch? It was accepted many kernels ago. Your questions on the original were based on false assumptions and facts which I've proven over and over. You are right in that I do not want to revisit those either. There is a patch that can solve the problem you are facing but yet you insist on the revert. This is very puzzling to me. You simply refuse to move this forward. That is your choice but revert is a no go for me.
Quoted 3 lines
Ok this is pretty confusing to someone not involved in the prior discussions. Could both of your stop attacking each other and start talking about the technical issues?
Quoted 7 lines
There's been multiple threads over this revert. This one is the latest Leon walked away from: https://patchwork.kernel.org/patch/9814367/
I'm simply asking for proof/evidence/facts to back up Leon's claims. Show me the code/stack trace/whatever and I will be happy to admit wrong publicly on the list and move on. I do have better things to do than to NACK a revert.
Hello Chien,
What Leon wrote, namely that calls that send netlink data from kernel to user space should be non-blocking makes sense to me. So please be more constructive than replying with "NAK".
Thanks,
Bart.--
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]
Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"
Quoted 24 lines · Bart Van Assche
Quoted 15 lines · Chien Tin Tung
Quoted 10 lines · Christopher Lameter
Quoted 7 lines · Chien Tin Tung
Why do I need to explain the original patch? It was accepted many kernels ago. Your questions on the original were based on false assumptions and facts which I've proven over and over. You are right in that I do not want to revisit those either. There is a patch that can solve the problem you are facing but yet you insist on the revert. This is very puzzling to me. You simply refuse to move this forward. That is your choice but revert is a no go for me.
Quoted 3 lines
Ok this is pretty confusing to someone not involved in the prior discussions. Could both of your stop attacking each other and start talking about the technical issues?
Quoted 7 lines
There's been multiple threads over this revert. This one is the latest Leon walked away from: https://patchwork.kernel.org/patch/9814367/
I'm simply asking for proof/evidence/facts to back up Leon's claims. Show me the code/stack trace/whatever and I will be happy to admit wrong publicly on the list and move on. I do have better things to do than to NACK a revert.
Quoted 4 lines
Hello Chien,
What Leon wrote, namely that calls that send netlink data from kernel to user space should be non-blocking makes sense to me.
That's been addressed in so many emails I won't rehash. If _no_one_ should block (it is actually a one shot retry with a timeout) sending Netlink message from kernel to user, why don't Leon or you patch that code out? All of it, not just ibnl_unicast().
Quoted 1 line
So please be more constructive than replying with "NAK".
I've sent so many emails (some you were CC'd), so I'm not sure how much more constructive I can be. BTW, did you see the one with my attempt at world peace (https://www.spinics.net/lists/linux-rdma/msg50591.html)?
Here are the relevant threads for people that are interested in participating in this discussion.
https://patchwork.kernel.org/patch/9814367/
https://patchwork.kernel.org/patch/9752855/
Chien
--
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): 20170801162135.GA240-TZeIlv3TuzOfrEmaQUPKxl95YUYmaKo1UNDiOz3kqAs@public.gmane.org
Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"
Quoted 15 lines · Chien Tin Tung
Quoted 2 lines · Bart Van Assche
What Leon wrote, namely that calls that send netlink data from kernel to user space should be non-blocking makes sense to me.
That's been addressed in so many emails I won't rehash. If _no_one_ should block (it is actually a one shot retry with a timeout) sending Netlink message from kernel to user, why don't Leon or you patch that code out? All of it, not just
ibnl_unicast().Quoted 1 line
So please be more constructive than replying with "NAK".
I've sent so many emails (some you were CC'd), so I'm not sure how much more constructive I can be. BTW, did you see the one with my attempt at world peace (https://www.spinics.net/lists/linux-rdma/msg50591.html)?
Quoted 6 lines
Here are the relevant threads for people that are interested in participating in this discussion.
Hello Chien,
Yes, I had read these e-mails but I do not agree with all of what was written in these e-mails. I'm not sure whether you are aware of the original design goal of the netlink mechanism? It was designed on purpose to be unreliable such that sending information from the kernel to user space would never block. If sending data over a netlink socket can block then analyzing whether or not any deadlocks can occur is only possible by examining kernel and user space together. If sending data over a netlink socket never blocks then a only the kernel has to be considered when analyzing whether or not a deadlock can occur. We do not want that userspace can cause the kernel to lock up. So it's very strange to me to see that today the kernel has facilities for both blocking and non-blocking sends over netlink sockets. My opinion here is that any client that needs reliable communication from kernel to user space should use another mechanism than netlink sockets. Additionally, since the kernel reports netlink socket buffer overflows to user space through the ENOBUFS error code, what's so hard about detecting that error code in user space and resynchronizing state if recv() fails with ENOBUFS?
Is your concern perhaps about the code in iwpmd/iwarp_pm_server.c? From what I see in process_iwpm_netlink_msg() it seems like all receive errors are treated equal and no state resynchronization occurs if the kernel reports ENOBUFS?
Bart.--
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]
Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"
Quoted 21 lines · Bart Van Assche
Quoted 20 lines · Chien Tin Tung
Quoted 2 lines · Bart Van Assche
What Leon wrote, namely that calls that send netlink data from kernel to user space should be non-blocking makes sense to me.
That's been addressed in so many emails I won't rehash. If _no_one_ should block (it is actually a one shot retry with a timeout) sending Netlink message from kernel to user, why don't Leon or you patch that code out? All of it, not just
ibnl_unicast().Quoted 1 line
So please be more constructive than replying with "NAK".
I've sent so many emails (some you were CC'd), so I'm not sure how much more constructive I can be. BTW, did you see the one with my attempt at world peace (https://www.spinics.net/lists/linux-rdma/msg50591.html)?
Here are the relevant threads for people that are interested in participating in this discussion.
Quoted 8 lines
Quoted 1 line
Hello Chien,
Yes, I had read these e-mails but I do not agree with all of what was written in these e-mails. I'm not sure whether you are aware of the original design goal of the netlink mechanism? It was designed on purpose to be unreliable such that sending information from the kernel to user space would never block.
Please show me a feference to Netlink design document/email on mailing list that specifically disallowed this?
Quoted 7 lines
If sending data over a netlink socket can block then analyzing whether or not any deadlocks can occur is only possible by examining kernel and user space together. If sending data over a netlink socket never blocks then a only the kernel has to be considered when analyzing whether or not a deadlock can occur. We do not want that userspace can cause the kernel to lock up. So it's very strange to me to see that today the kernel has facilities for both blocking and non-blocking sends over netlink sockets.
So here we go again. I will rehash my arguments.
1) There is NO deadlock with ibnl_unicast(). It is single shot retry with timeout. As I've requested before, people should stop using terms that are NOT applicable to this situation. You can make other arguments but not this one.
2) With the patch from Mustafa it will separate out messages, one with the retry and the other without, as designed in Netlink. what's wrong with this solution? Has Leon tried the patch? Does it not work for his situation?
Quoted 6 lines
My opinion here is that any
client that needs reliable communication from kernel to user space should use another mechanism than netlink sockets. Additionally, since the kernel reports netlink socket buffer overflows to user space through the ENOBUFS error code, what's so hard about detecting that error code in user space and resynchronizing state ifrecv()fails with ENOBUFS?
Incorrect analysis on how the message is triggered. The message originates from kernel to user. It is not trying to guarantee delivery as it is single retry with timeout. Yes, I've stated these facts in previous threads.
Quoted 5 lines
Is your concern perhaps about the code in
iwpmd/iwarp_pm_server.c? From what I see inprocess_iwpm_netlink_msg()it seems like all receive errors are treated equal and no state resynchronization occurs if the kernel reports ENOBUFS?
you need to visit the kernel code flow not user. I've laid out the code flow for everyone to see in a previous email, not doing it again.
Chien
--
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): 20170801171454.GA8484-TZeIlv3TuzOfrEmaQUPKxl95YUYmaKo1UNDiOz3kqAs@public.gmane.org
Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"
Quoted 8 lines · Chien Tin Tung
Quoted 4 lines · Bart Van Assche
Yes, I had read these e-mails but I do not agree with all of what was written in these e-mails. I'm not sure whether you are aware of the original design goal of the netlink mechanism? It was designed on purpose to be unreliable such that sending information from the kernel to user space would never block.
Please show me a feference to Netlink design document/email on mailing list that specifically disallowed this?
As you probably know kernel developers do not write design documents. But there is plenty of evidence on the web that the netlink mechanism was designed to be unreliable. A quote from a Linux Journal article from 2005
(http://www.linuxjournal.com/article/7356): "Netlink is asynchronous because, as with any other socket API, it provides a socket queue to smooth the burst of messages."
Quoted 1 line
From LWN.net article from 2005 (https://lwn.net/Articles/131802/):
"netlink is an unreliable service".
Quoted 1 line
From RFC 3549, co-authored by Alexey Kuznetsov, one of the netlink authors
(https://tools.ietf.org/html/rfc3549): "By default, however, Netlink provides an unreliable communication."
Quoted 1 line
From the netlink man page (http://man7.org/linux/man-pages/man7/netlink.7.html):
"However, reliable transmissions from kernel to user are impossible in any case. The kernel can't send a netlink message if the socket buffer is full: the message will be dropped and the kernel and the user-space process will no longer have the same view of kernel state. It is up to the application to detect when this happens (via the ENOBUFS error returned by recvmsg(2)) and resynchronize."
Bart.--
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]
Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"
Quoted 10 lines · Bart Van Assche
Quoted 8 lines · Chien Tin Tung
Quoted 4 lines · Bart Van Assche
Yes, I had read these e-mails but I do not agree with all of what was written in these e-mails. I'm not sure whether you are aware of the original design goal of the netlink mechanism? It was designed on purpose to be unreliable such that sending information from the kernel to user space would never block.
Please show me a feference to Netlink design document/email on mailing list that specifically disallowed this?
Quoted 6 lines
As you probably know kernel developers do not write design documents. But there is plenty of evidence on the web that the netlink mechanism was designed to be unreliable. A quote from a Linux Journal article from 2005
(http://www.linuxjournal.com/article/7356): "Netlink is asynchronous because, as with any other socket API, it provides a socket queue to smooth the burst of messages."
So what happens when the queue is full? you can fail at that point or you can choose the 1-shot retry with timeout as we have done. What is _wrong_ with the 1-shot retry?
Quoted 2 lines
From LWN.net article from 2005 (https://lwn.net/Articles/131802/):
"netlink is an unreliable service".
fine again, already stated ibnl_unicast is not a guaranteed delivery of Netlink messages. I can only say this so many times.
Quoted 3 lines
From RFC 3549, co-authored by Alexey Kuznetsov, one of the netlink authors
(https://tools.ietf.org/html/rfc3549): "By default, however, Netlink provides an unreliable communication."
Same as above.
Quoted 6 lines
From the netlink man page (http://man7.org/linux/man-pages/man7/netlink.7.html):
"However, reliable transmissions from kernel to user are impossible in any case. The kernel can't send a netlink message if the socket buffer is full: the message will be dropped and the kernel and the user-space process will no longer have the same view of kernel state. It is up to the application to detect when this happens (via the ENOBUFS error returned by recvmsg(2)) and resynchronize."
Same as above.
It is so convenient for you and Leon to delete my questions that you don't want to address.
So, here they are again.
The Netlink message we want 1-shot retry is triggered from kernel to user, not user->kernel->user. Please get this right. Mustafa's patch would separate out messages that's user->kernel->user. What's wrong with that solution? [rehash] The original patch to add 1-shot retry was to split ibnl_unicast into two, one with retry, the other without, which Leon shot down. So please don't ask how we ended up here.
I see two path out of this.
1) Either Leon/you patch out all blocking Netlink code from the kernel, including the 1-shot retry mechanism ibln_unicast is using. I promise I won't comment on the patch. I will let Netlink maintainer handle that matter.
2) Accept Mustafa's patch.
It is time to move on from this discussion.
Chien
--
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): 20170801175236.GA14048-TZeIlv3TuzOfrEmaQUPKxl95YUYmaKo1UNDiOz3kqAs@public.gmane.org
Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"
Quoted 18 lines · Chien Tin Tung
Quoted 10 lines · Bart Van Assche
Quoted 8 lines · Chien Tin Tung
Quoted 4 lines · Bart Van Assche
Yes, I had read these e-mails but I do not agree with all of what was written in these e-mails. I'm not sure whether you are aware of the original design goal of the netlink mechanism? It was designed on purpose to be unreliable such that sending information from the kernel to user space would never block.
Please show me a feference to Netlink design document/email on mailing list that specifically disallowed this?
Quoted 6 lines
As you probably know kernel developers do not write design documents. But there is plenty of evidence on the web that the netlink mechanism was designed to be unreliable. A quote from a Linux Journal article from 2005
(http://www.linuxjournal.com/article/7356): "Netlink is asynchronous because, as with any other socket API, it provides a socket queue to smooth the burst of messages."
Quoted 3 lines
So what happens when the queue is full? you can fail at that point or you can choose the 1-shot retry with timeout as we have done. What is _wrong_ with the 1-shot retry?
Why is the one-shot retry necessary? What aspect of the portmapper design makes this one-shot retry necessary? Why is this one-shot retry only necessary for the portmapper and not for any other Netlink client?
Bart.--
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]
Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"
Quoted 19 lines · Bart Van Assche
Quoted 18 lines · Chien Tin Tung
Quoted 16 lines · Bart Van Assche
Quoted 8 lines · Chien Tin Tung
Quoted 4 lines · Bart Van Assche
Yes, I had read these e-mails but I do not agree with all of what was written in these e-mails. I'm not sure whether you are aware of the original design goal of the netlink mechanism? It was designed on purpose to be unreliable such that sending information from the kernel to user space would never block.
Please show me a feference to Netlink design document/email on mailing list that specifically disallowed this?
As you probably know kernel developers do not write design documents. But there is plenty of evidence on the web that the netlink mechanism was designed to be unreliable. A quote from a Linux Journal article from 2005
(http://www.linuxjournal.com/article/7356): "Netlink is asynchronous because, as with any other socket API, it provides a socket queue to smooth the burst of messages."
Quoted 7 lines
Quoted 3 lines
So what happens when the queue is full? you can fail at that point or you can choose the 1-shot retry with timeout as we have done. What is _wrong_ with the 1-shot retry?
Why is the one-shot retry necessary? What aspect of the portmapper design makes this one-shot retry necessary? Why is this one-shot retry only necessary for the portmapper and not for any other Netlink client?
You are really asking why the 1-shot retry code is in Netlink. We didn't code it, we are simply using it with ibnl_unicast. It being used as designed, when the queue is full, try it one more time within the timeout parameter. Other clients can use it too, I'm not stopping them.
Chien
--
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
Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"
In reply to a message not in the archiveQuoted 15 lines · Chien Tin Tung
Quoted 10 lines · Christopher Lameter
Quoted 7 lines · Chien Tin Tung
Why do I need to explain the original patch? It was accepted many kernels ago. Your questions on the original were based on false assumptions and facts which I've proven over and over. You are right in that I do not want to revisit those either. There is a patch that can solve the problem you are facing but yet you insist on the revert. This is very puzzling to me. You simply refuse to move this forward. That is your choice but revert is a no go for me.
Quoted 3 lines
Ok this is pretty confusing to someone not involved in the prior discussions. Could both of your stop attacking each other and start talking about the technical issues?
Quoted 7 lines
There's been multiple threads over this revert. This one is the latest Leon walked away from: https://patchwork.kernel.org/patch/9814367/
I'm simply asking for proof/evidence/facts to back up Leon's claims. Show me the code/stack trace/whatever and I will be happy to admit wrong publicly on the list and move on. I do have better things to do than to NACK a revert.
Well, I think the design of the netlink protocol in iwpmd is pretty strange, but audit netlink is the other major user of blocking netlink_uncast to achive some reliability, so it should be workable here as well.
Presumably the iwpmd kernel side is designed to handle the locking right to avoid deadlocks.
I'm not sure why there is so much noise about this - yes, iwpmd is really weird, but it is a UAPI, we can't change it and we can't demand they change. For better or worse the protocol is based on 'near-reliable' netlink delivery of messages and we are stuck with that.
Leon is also right that every other user of netlink_unicast in rdma should be using the non-blocking version.
So, the above patch seems like a sensible approach to me..
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): 20170801195840.GC31205-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org
Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"
Quoted 24 lines · Jason Gunthorpe
Quoted 15 lines · Chien Tin Tung
Quoted 10 lines · Christopher Lameter
Quoted 7 lines · Chien Tin Tung
Why do I need to explain the original patch? It was accepted many kernels ago. Your questions on the original were based on false assumptions and facts which I've proven over and over. You are right in that I do not want to revisit those either. There is a patch that can solve the problem you are facing but yet you insist on the revert. This is very puzzling to me. You simply refuse to move this forward. That is your choice but revert is a no go for me.
Quoted 3 lines
Ok this is pretty confusing to someone not involved in the prior discussions. Could both of your stop attacking each other and start talking about the technical issues?
Quoted 7 lines
There's been multiple threads over this revert. This one is the latest Leon walked away from: https://patchwork.kernel.org/patch/9814367/
I'm simply asking for proof/evidence/facts to back up Leon's claims. Show me the code/stack trace/whatever and I will be happy to admit wrong publicly on the list and move on. I do have better things to do than to NACK a revert.
Quoted 4 lines
Well, I think the design of the netlink protocol in iwpmd is pretty strange, but audit netlink is the other major user of blocking
netlink_uncastto achive some reliability, so it should be workable here as well.
Agreed, somewhat. I'm all for making things better but the basis of any conversation needs to start with correct assumptions.
Quoted 2 lines
Presumably the iwpmd kernel side is designed to handle the locking right to avoid deadlocks.
Please stop using the word deadlock in relation to usage of 1-shot retry in ibnl_unicast. It does not and will not cause deadlock period. It will wait for a buffer within timeout value but that's it.
Quoted 8 lines
I'm not sure why there is so much noise about this - yes, iwpmd is really weird, but it is a UAPI, we can't change it and we can't demand they change. For better or worse the protocol is based on 'near-reliable' netlink delivery of messages and we are stuck with that.
Leon is also right that every other user of
netlink_unicastin rdma should be using the non-blocking version.
In Mustafa's patch, there are calls to both versions of ibnl_unicast. There are situations where the retry version of ibnl_unicast is not desirable, note I didn't say wrong.
Here is the reality.
The retry version of ibnl_unicast will only wait when there is no buffer to send the Netlink message. It does not interfere with other Netlink calls. For that matter, other calls to transmit a Netlink message will fail as there are no buffers available. If a buffer does not become available within timeout parameter, you have bigger problems.
Quoted 1 line
So, the above patch seems like a sensible approach to me..
Finally. Thank you.
Chien
--
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): 20170801203815.GA4620-TZeIlv3TuzOfrEmaQUPKxl95YUYmaKo1UNDiOz3kqAs@public.gmane.org
Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"
Quoted 6 lines · Chien Tin Tung
Quoted 2 lines
Presumably the iwpmd kernel side is designed to handle the locking right to avoid deadlocks.
Please stop using the word deadlock in relation to usage of 1-shot retry in
ibnl_unicast. It does not and will not cause deadlock period. It will wait for a buffer within timeout value but that's it.
I think you are arguing semantics here.
My understanding with netlink (and it has been years since I've looked) is that there is way for netlink callbacks to be run within the context of the user thread calling send(). In this case calling blocking netlink_unicast within such a callback will 'deadlock', as netlink_unicast waits for the current thread to call recv(), which can never happen.
We still call this a deadlock even though the sndtimer rescues the overall system, as the system was in a deadlocked state for a time.
Quoted 6 lines
Quoted 2 lines
Leon is also right that every other user of
netlink_unicastin rdma should be using the non-blocking version.In Mustafa's patch, there are calls to both versions of
ibnl_unicast. There are situations where the retry version ofibnl_unicastis not desirable, note I didn't say wrong.
I haven't studied closely, but if any of those blocking netlink_unicasts are in netlink callbacks, it would need very careful inspection to determine if that is OK.
Quoted 2 lines
buffers available. If a buffer does not become available within timeout parameter, you have bigger problems.
Well.. It is problematic to create that situation in the first place.
Usually you would never run unsolicited netlink traffic and request/response traffic over the same socket. Having the kernel randomly drop responses because the sndbuf is full of unsolicited traffic is really problematic.
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
Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"
In reply to a message not in the archiveQuoted 4 lines · Jason Gunthorpe
I'm not sure why there is so much noise about this - yes, iwpmd is really weird, but it is a UAPI, we can't change it and we can't demand they change.
If you claim that it is UAPI change, we MUST revert this patch, because reverted patch CHANGED UAPI.
Thanks
A message that is not in the archive (sent off-list or not yet received): 20170802034438.GV13672-U/[email protected]
Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"
Quoted 8 lines · Leon Romanovsky
Quoted 4 lines · Jason Gunthorpe
I'm not sure why there is so much noise about this - yes, iwpmd is really weird, but it is a UAPI, we can't change it and we can't demand they change.
If you claim that it is UAPI change, we MUST revert this patch, because reverted patch CHANGED UAPI.
That is not how I read it.. The UAPI was intended to be lossless and there was a kernel bug that made it more lossy than expected, that is what that original patch was addressing.
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): 20170802155856.GA21208-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org
Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"
Quoted 7 lines · Jason Gunthorpe
Quoted 6 lines · Leon Romanovsky
Quoted 4 lines · Jason Gunthorpe
I'm not sure why there is so much noise about this - yes, iwpmd is really weird, but it is a UAPI, we can't change it and we can't demand they change.
Quoted 6 lines
Quoted 2 lines
If you claim that it is UAPI change, we MUST revert this patch, because reverted patch CHANGED UAPI.
That is not how I read it.. The UAPI was intended to be lossless and there was a kernel bug that made it more lossy than expected, that is what that original patch was addressing.
The catch 22 here is in the fact that there is no kernel bug. I assume and according to Bart's questions (but better to ask him) he thinks the same, that the bug is in protocol layer and/or user space part. There is no visible kernel bug.
This is exactly what we are asking from Chien to provide the details.
Thanks
Quoted 2 lines
Jason
A message that is not in the archive (sent off-list or not yet received): 20170802162938.GC13672-U/[email protected]
Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"
Quoted 11 lines · Leon Romanovsky
Quoted 10 lines · Jason Gunthorpe
Quoted 8 lines · Leon Romanovsky
Quoted 4 lines · Jason Gunthorpe
I'm not sure why there is so much noise about this - yes, iwpmd is really weird, but it is a UAPI, we can't change it and we can't demand they change.
If you claim that it is UAPI change, we MUST revert this patch, because reverted patch CHANGED UAPI.
Quoted 8 lines
Quoted 3 lines
That is not how I read it.. The UAPI was intended to be lossless and there was a kernel bug that made it more lossy than expected, that is what that original patch was addressing.
The catch 22 here is in the fact that there is no kernel bug. I assume and according to Bart's questions (but better to ask him) he thinks the same, that the bug is in protocol layer and/or user space part. There is no visible kernel bug.
No, I disagree.. It is pretty clear how iwpm works, it it pushes unsolicted messaged directly to the userspace daemon from the kernel and expects the daemon to receive those messages.
With such a scheme it is really important for the kernel to do everything it can to minimize the risk of message loss, and using the blocking sending for unsolicited messages is certainly part of it - that is what audit does for instance.
The discussion really got into the weeds when people brought up O_NONBLOCK or ENOBUFS, or any other user space change as that has nothing to do with pushing unsolicited messages from the kernel to userspace.
Arguing that is a 'protocol bug' doesn't make much sense, the protocol is a uAPI, so it is up to the kernel to provide the best implementation possible, which in this case means working to minimize loss of the messages..
Noting again, that this is *ONLY* talking about the unsolicted messages the iwpm sends toward userspace. IMHO, use of blocking send in other contexts, such as dump callbacks, is an error.
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): 20170802164553.GA31901-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org
Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"
Quoted 14 lines · Jason Gunthorpe
No, I disagree.. It is pretty clear how iwpm works, it it pushes unsolicted messaged directly to the userspace daemon from the kernel and expects the daemon to receive those messages.
With such a scheme it is really important for the kernel to do everything it can to minimize the risk of message loss, and using the blocking sending for unsolicited messages is certainly part of it - that is what audit does for instance.
The discussion really got into the weeds when people brought up
O_NONBLOCKor ENOBUFS, or any other user space change as that has nothing to do with pushing unsolicited messages from the kernel to userspace.
Quoted 8 lines
Arguing that is a 'protocol bug' doesn't make much sense, the protocol is a uAPI, so it is up to the kernel to provide the best implementation possible, which in this case means working to minimize loss of the messages..
Noting again, that this is *ONLY* talking about the unsolicted messages the iwpm sends toward userspace. IMHO, use of blocking send in other contexts, such as dump callbacks, is an error.
Hello Jason,
Although I do not object against the "RDMA/core: Add wait/retry version of ibnl_unicast" patch, I hope that you realize that it is an ugly hack instead of a proper fix. Anything that makes user space wait longer than the socket timeout, e.g. heavy swapping activity or running the user space software under a debugger, will cause delivery of the netlink message from kernel to user to fail anyway.
Bart.--
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]
Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"
Quoted 6 lines · Bart Van Assche
Although I do not object against the "RDMA/core: Add wait/retry version of
ibnl_unicast" patch, I hope that you realize that it is an ugly hack instead of a proper fix. Anything that makes user space wait longer than the socket timeout, e.g. heavy swapping activity or running the user space software under a debugger, will cause delivery of the netlink message from kernel to user to fail anyway.
Yes, I assume the iwpm people are aware of this as well, this is why I used the phrase 'minimize loss'.
I'm not sure I'd call it a hack though. There is no proper fix here, messages from kernel to user space must be delivered and processed for iwpm to function as designed. Maximizing the chance of delivering unsolicited messages via blocking netlink_unicast is going to be a necessary component of any design...
The big point in all of this discussion is that none of this allows iwpmd to ignore lost messages and it needs to have a sane well thought out process for resynchronizing with the kernel. Particularly, the flow that causes sockets to closed must have some kind of resync.
But even if all that loss resynchronization works perfectly, it is still very desirable to avoid triggering it!
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): 20170802170823.GA32513-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org
Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"
Quoted 18 lines · Jason Gunthorpe
Quoted 6 lines · Bart Van Assche
Although I do not object against the "RDMA/core: Add wait/retry version of
ibnl_unicast" patch, I hope that you realize that it is an ugly hack instead of a proper fix. Anything that makes user space wait longer than the socket timeout, e.g. heavy swapping activity or running the user space software under a debugger, will cause delivery of the netlink message from kernel to user to fail anyway.Yes, I assume the iwpm people are aware of this as well, this is why I used the phrase 'minimize loss'.
I'm not sure I'd call it a hack though. There is no proper fix here, messages from kernel to user space must be delivered and processed for iwpm to function as designed. Maximizing the chance of delivering unsolicited messages via blocking
netlink_unicastis going to be a necessary component of any design...
Quoted 7 lines
The big point in all of this discussion is that none of this allows iwpmd to ignore lost messages and it needs to have a sane well thought out process for resynchronizing with the kernel. Particularly, the flow that causes sockets to closed must have some kind of resync.
But even if all that loss resynchronization works perfectly, it is still very desirable to avoid triggering it!
Hello Jason,
Because it is not possible to make the current iwpm netlink code work 100% reliably, my proposal is to add a new and redesigned interface for iwpm communication between user space and kernel and to deprecate the current API.
Bart.--
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]
Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"
Quoted 2 lines · Bart Van Assche
Because it is not possible to make the current iwpm netlink code work 100% reliably,
Well, I don't know enough about what iwpm has exactly done in those details to be sure of that statement.. I'm not sure what the message loss strategy is. It looks like loss on connect() just refuses the connection, which seems fine, but loss on close() looks troubled? Not sure.
I'm not really inclined to look into iwpm in any detail, but I really want to see Leon's patch series progress, so overcoming that hurdle by using the patch Chien pointed out seems perfectly fine to me.
Quoted 3 lines
my proposal is to add a new and redesigned interface for iwpm communication between user space and kernel and to deprecate the current API.
What would you do differently?
And even if this is the best path, the use of blocking send in the compat uAPI code is *still* very appropriate.
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
Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"
In reply to a message not in the archiveQuoted 25 lines · Leon Romanovsky
Quoted 12 lines · Chien Tin Tung
Quoted 8 lines · Leon Romanovsky
From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
The commit
cea05eadded0("IB/core: Add flow control to the portmapper netlink calls")
changed netlink to be blocked for all RDMA clients. This workaround
worked perfectly for portmapper, but is not correct for the wholeNETLINK_RDMAfamily.Leon,
Quoted 9 lines
I've already told you I'm opposed to the revert. There is a patch that will work
with your usage ofibnl_unicast()but you chose to abandon that discussion on June 29, 2017
(RDMA/core: Add wait/retry version ofibnl_unicast). Either you step up with good sound
technical evidence to convince me that patch won't work for you or you stop trying to
break existing functionality with this revert.Chien,
Quoted 8 lines
You never explained us what exactly your original patch fixed and why it
should be fixed in kernel and not in user space. I saw that our discussion wasn't useful and brought bad blood instead of good will, so I stepped out in waited for RDMA maintainer (Doug) step in.I never heard anything from Doug on the matter, so proceeded and resubmitted it.
What makes you think *I* want to be in the middle of a school yard squabble like this?
Regardless, here's what I think:
1) The blocking/timeout functionality that Chien is using is in the netlink core, not something he grafted onto netlink himself. Since it's a core netlink feature, I see no reason he shouldn't be allowed to use it.
2) We don't break API for user space programs. Reverting this could have a significant deleterious influence on programs already in the wild. While those programs should resync on a failure, clearly, failure is much more frequent without this patch than with it, and as a resync can be expensive, it is a judgment call whether it is preferable to resync more frequently because of dropped packets, or utilize a backlog queue to reduce the frequency of resyncs. Clearly, Intel is of the position that the backlog queue is worth avoiding the resyncs.
3) Chien submitted a patch that would allow just the iwpmd to get the behavior they find preferable, while allowing Leon's new code to get the behavior Leon wants.
Given that #1 and #2 are true and that #3 provides a reasonable solution, the revert is rejected.
Signature · 8 lines
Doug Ledford <[email protected]> GPG KeyID: B826A3330E572FDD Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD -- 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): 1501696661.109555.6.camel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"
Quoted 32 lines · Doug Ledford
Quoted 31 lines · Leon Romanovsky
Quoted 12 lines · Chien Tin Tung
Quoted 8 lines · Leon Romanovsky
From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
The commit
cea05eadded0("IB/core: Add flow control to the portmapper netlink calls")
changed netlink to be blocked for all RDMA clients. This workaround
worked perfectly for portmapper, but is not correct for the wholeNETLINK_RDMAfamily.Leon,
Quoted 9 lines
I've already told you I'm opposed to the revert. There is a patch that will work
with your usage ofibnl_unicast()but you chose to abandon that discussion on June 29, 2017
(RDMA/core: Add wait/retry version ofibnl_unicast). Either you step up with good sound
technical evidence to convince me that patch won't work for you or you stop trying to
break existing functionality with this revert.Chien,
You never explained us what exactly your original patch fixed and why it
should be fixed in kernel and not in user space. I saw that our discussion wasn't useful and brought bad blood instead of good will, so I stepped out in waited for RDMA maintainer (Doug) step in.
Quoted 5 lines
Quoted 2 lines
I never heard anything from Doug on the matter, so proceeded and resubmitted it.
What makes you think *I* want to be in the middle of a school yard squabble like this?
It is not "I" want, but "I" expect and "I" demand from the maintainer. It is his responsibility to drive to the resolution for the important core patches.
This revert was discussed for something like month+, as an outcome of it, Chien reviewed and sent much more improved version of their original patch, without socket timeout and with minimal blocking places.
So please, don't say "school yard", especially after your silence.
Thanks
A message that is not in the archive (sent off-list or not yet received): 20170802185405.GE13672-U/[email protected]
Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"
Quoted 40 lines · Leon Romanovsky
Quoted 39 lines · Doug Ledford
Quoted 38 lines · Leon Romanovsky
Quoted 14 lines · Chien Tin Tung
Quoted 9 lines · Leon Romanovsky
From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
The commit
cea05eadded0("IB/core: Add flow control to the portmapper netlink calls")
changed netlink to be blocked for all RDMA clients. This workaround
worked perfectly for portmapper, but is not correct for the wholeNETLINK_RDMAfamily.Leon,
Quoted 12 lines
I've already told you I'm opposed to the revert. There is a patch
that will work
with your usage ofibnl_unicast()but you chose to abandon that discussion on June 29, 2017
(RDMA/core: Add wait/retry version ofibnl_unicast). Either you
step up with good sound
technical evidence to convince me that patch won't work for you or
you stop trying to
break existing functionality with this revert.Chien,
You never explained us what exactly your original patch fixed and why
it
should be fixed in kernel and not in user space. I saw that our discussion wasn't useful and brought bad blood instead of good will,
so I stepped out in waited for RDMA maintainer (Doug) step in.
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
I never heard anything from Doug on the matter, so proceeded and resubmitted it.
What makes you think *I* want to be in the middle of a school yard squabble like this?
It is not "I" want, but "I" expect and "I" demand from the maintainer.
I'm a big fan of the movie Draft Day. You may not have ever seen it, but I really enjoy all of the things that happen in the course of one day. In that movie, Vontae Mack is afraid he won't get selected 7th by Sonny Weaver Jr. When Sonny Weaver Jr. trades up for the 1st pick, Vontae assumes it's because he's going to try and take the popular pick, and so Vontae takes to his twitter account and tells Sonny Weaver Jr. off. Sonny gives Vontae a call, in which he tells Vontae that he needs to put his twitter account down and take a chill pill, because "as much as going to twitter is your God given right, General Managers hate that shit because now they know that you'll put whatever innane thought goes through your mind out there for the entire world to see." (or something like that, it's from memory).
You may be wondering what my point here is. It's simple. It may be your God given right to force a maintainer to make a call like this, but maintainers hate that shit, because it's usually a sign of dysfunction in the community and failure to work cooperatively to arrive at a communally approved resolution. What's more, it puts the maintainer in a bad position of having to deal with possible political crap and everything else. So, yes, it's your God given right, but it better be your last resort.
Quoted 2 lines
It is his responsibility to drive to the resolution for the important core patches.
This was really only a core patch of minor importance to begin with, and once Chien wrote a patch to give you what you wanted without breaking the existing user space packages, it really became a trivial core patch. Hardly worthy of pulling the "I demand you make a maintainer call" card.
Quoted 4 lines
This revert was discussed for something like month+, as an outcome of it, Chien reviewed and sent much more improved version of their original
patch, without socket timeout and with minimal blocking places.
That would have been a perfect moment for you to work with Chien to take his patch as the first of your series instead of sticking to the revert.
Quoted 1 line
So please, don't say "school yard",
I stand by my statement. So many school yard arguments make no real sense, and given you had every opportunity to resolve this problem with a community viable alternative patch, and in so doing would have actually fulfilled the edict that "we don't break user space" instead of violating it as you were doing, the metaphor seems entirely appropriate.
Quoted 1 line
especially after your silence.
I want to address this separate from everything else.
With as much hostility and accusations as I saw fly on the original submission, several things happened:
1) The submission got bumped to the bottom of my queue because settling squables is not the sort of thing maintainers want to deal with.
2) The submission got bumped to the bottom of my queue so you would have a chance to work things out and come to a mutually agreed solution. Such resolution is in the best interest of both the community and the technical correctness of the fix as, generally, an agreed upon fix will have had more community input and refinement prior to being accepted.
3) The submission will get put on hold for a short while regardless of points 1 and 2 just so you have a moment to settle down, rethink things a little bit, and decide if you really want to pull that "put the maintainer in this position" trigger. Have you really examined all the possible solutions? Is there really no superior alternative? Is there really no way to come to agreement with the other community member(s) you are disagreeing with?
In the end, there will always be those things that a maintainer will simply need to make a call on because there won't be a clear right or wrong side and it just becomes a judgment call. But I don't think this one even came close to being that unclear, so again I'm confused by your insistence on going this route instead of just working things out with Chien.
Signature · 8 lines
Doug Ledford <[email protected]> GPG KeyID: B826A3330E572FDD Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD -- 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): 1501719575.117042.4.camel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"
Quoted 63 lines · Doug Ledford
Quoted 40 lines · Leon Romanovsky
Quoted 39 lines · Doug Ledford
Quoted 38 lines · Leon Romanovsky
Quoted 14 lines · Chien Tin Tung
Quoted 9 lines · Leon Romanovsky
From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
The commit
cea05eadded0("IB/core: Add flow control to the portmapper netlink calls")
changed netlink to be blocked for all RDMA clients. This workaround
worked perfectly for portmapper, but is not correct for the wholeNETLINK_RDMAfamily.Leon,
Quoted 12 lines
I've already told you I'm opposed to the revert. There is a patch
that will work
with your usage ofibnl_unicast()but you chose to abandon that discussion on June 29, 2017
(RDMA/core: Add wait/retry version ofibnl_unicast). Either you
step up with good sound
technical evidence to convince me that patch won't work for you or
you stop trying to
break existing functionality with this revert.Chien,
You never explained us what exactly your original patch fixed and why
it
should be fixed in kernel and not in user space. I saw that our discussion wasn't useful and brought bad blood instead of good will,
so I stepped out in waited for RDMA maintainer (Doug) step in.Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
I never heard anything from Doug on the matter, so proceeded and resubmitted it.
What makes you think *I* want to be in the middle of a school yard squabble like this?
It is not "I" want, but "I" expect and "I" demand from the maintainer.
I'm a big fan of the movie Draft Day. You may not have ever seen it, but I really enjoy all of the things that happen in the course of one day. In that movie, Vontae Mack is afraid he won't get selected 7th by Sonny Weaver Jr. When Sonny Weaver Jr. trades up for the 1st pick, Vontae assumes it's because he's going to try and take the popular pick, and so Vontae takes to his twitter account and tells Sonny Weaver Jr. off. Sonny gives Vontae a call, in which he tells Vontae that he needs to put his twitter account down and take a chill pill, because "as much as going to twitter is your God given right, General Managers hate that shit because now they know that you'll put whatever innane thought goes through your mind out there for the entire world to see." (or something like that, it's from memory).
Quoted 8 lines
You may be wondering what my point here is. It's simple. It may be your God given right to force a maintainer to make a call like this, but maintainers hate that shit, because it's usually a sign of dysfunction in the community and failure to work cooperatively to arrive at a communally approved resolution. What's more, it puts the maintainer in a bad position of having to deal with possible political crap and everything else. So, yes, it's your God given right, but it better be your last resort.
Are you kidding me? Reply to the series from the maintainer that "it is postponed till both sides are happy" is too much for you?
"Politics" - it is so sticky word here. Every disagreement in the mailing list immediately claimed "for politic reasons" and everyone is hiding in the bushes.
So yes, the community doesn't work, and I don't see desire to fix it from the all participants of this mailing list.
Quoted 9 lines
Quoted 3 lines
It is his responsibility to drive to the resolution for the important core patches.
This was really only a core patch of minor importance to begin with, and once Chien wrote a patch to give you what you wanted without breaking the existing user space packages, it really became a trivial core patch. Hardly worthy of pulling the "I demand you make a maintainer call" card.
No, Chien never explained why he needs this fix and what this fix is actually fixing.
Quoted 9 lines
Quoted 5 lines
This revert was discussed for something like month+, as an outcome of it, Chien reviewed and sent much more improved version of their original
patch, without socket timeout and with minimal blocking places.That would have been a perfect moment for you to work with Chien to take his patch as the first of your series instead of sticking to the revert.
I can't work on the solution if other side didn't explain why it is needed.
Quoted 9 lines
Quoted 2 lines
So please, don't say "school yard",
I stand by my statement. So many school yard arguments make no real sense, and given you had every opportunity to resolve this problem with a community viable alternative patch, and in so doing would have actually fulfilled the edict that "we don't break user space" instead of violating it as you were doing, the metaphor seems entirely appropriate.
And the fact that the reverted patch broke the user space behavior before, didn't bother you?
Quoted 8 lines
Quoted 2 lines
especially after your silence.
I want to address this separate from everything else.
With as much hostility and accusations as I saw fly on the original submission, several things happened:
Quoted 15 lines
1) The submission got bumped to the bottom of my queue because settling squables is not the sort of thing maintainers want to deal with.
2) The submission got bumped to the bottom of my queue so you would have a chance to work things out and come to a mutually agreed solution. Such resolution is in the best interest of both the community and the technical correctness of the fix as, generally, an agreed upon fix will have had more community input and refinement prior to being accepted.
3) The submission will get put on hold for a short while regardless of points 1 and 2 just so you have a moment to settle down, rethink things a little bit, and decide if you really want to pull that "put the maintainer in this position" trigger. Have you really examined all the possible solutions? Is there really no superior alternative? Is there really no way to come to agreement with the other community member(s) you are disagreeing with?
And did you go to the mailing list and write something like that "the series on hold till solution will be found", like it was with hfi1-vnic and ipoib?
The Answer is no, you didn't do it, you silently ignored discussion, understood that the series is stuck and continued down the road.
Quoted 7 lines
In the end, there will always be those things that a maintainer will simply need to make a call on because there won't be a clear right or wrong side and it just becomes a judgment call. But I don't think this one even came close to being that unclear, so again I'm confused by your insistence on going this route instead of just working things out with Chien.
Because, I truly believe that they proposed the nasty hack, which doesn't fix the real problem - inability to deal with losses of netlink messages.
And I realized it after I started to work on RDMA netlink and read RFC and design documents and assumptions about netlink.
In my opinion, it is one of the small examples why RDMA subsystem is so hated. It is full of "custom" solutions to known problems and many of those "custom" solutions are mix of hacks and band aids.
There is no need to reply on it.
It is my personal opinion and my view of the RDMA subsystem as a newcomer.
I'm OOO till Monday.
Happy weekends.
Quoted 7 lines
--
Doug Ledford <[email protected]>GPG KeyID: B826A3330E572FDD Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD
A message that is not in the archive (sent off-list or not yet received): 20170803051032.GF13672-U/[email protected]
Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"
Quoted 7 lines · Leon Romanovsky
Quoted 2 lines · Doug Ledford
Quoted 1 line · Leon Romanovsky
Are you kidding me? Reply to the series from the maintainer that "it is
postponed till both sides are happy" is too much for you?
I was a bit busy the last few weeks fixing a mess of my own. And I'm still extremely busy working on getting our test cluster in Red Hat back operational....it's been down now for almost four weeks in total (well, planning and downtime has now extended to four weeks, only three weeks have been downtime). I don't delete emails from my linux-rdma folder until they've been fully processed and won't be needed any more.
Between the problems I created for myself by missing the merge window,
and the downtime of the Red Hat cluster I've been working on, I have 1500 emails in that folder. So, yes, I punted on what looked like a squabble and worked on other stuff.
Quoted 13 lines
Quoted 5 lines
Quoted 4 lines
It is his responsibility to drive to the resolution for the important
core patches.Quoted 7 lines
This was really only a core patch of minor importance to begin with,
and once Chien wrote a patch to give you what you wanted without breaking the existing user space packages, it really became a trivial
core patch. Hardly worthy of pulling the "I demand you make a maintainer call" card.
Quoted 2 lines
No, Chien never explained why he needs this fix and what this fix is actually fixing.
I understood his explanation. He wasn't as clear as he could have been, but then again he's ESL just like you (English as Second Language, it's the term they use in my wife's job for situations like this). I'd be happy to explain if it will help.
Quoted 7 lines
Quoted 7 lines
Quoted 6 lines
This revert was discussed for something like month+, as an outcome of
it, Chien reviewed and sent much more improved version of their original
patch, without socket timeout and with minimal blocking places.
Quoted 7 lines
Quoted 4 lines
That would have been a perfect moment for you to work with Chien to take his patch as the first of your series instead of sticking to the
revert.I can't work on the solution if other side didn't explain why it is needed.
He's ESL just like you are, with both of you I expect unintentional shortcomings in explanations, and when that happens I attempt to clarify for the sake of all parties. He tried, as best he could, to give you what you asked for. I got it, if you need help understanding it I'd be glad to help.
Quoted 13 lines
Quoted 12 lines
Quoted 2 lines
So please, don't say "school yard",
I stand by my statement. So many school yard arguments make no real
sense, and given you had every opportunity to resolve this problem with
a community viable alternative patch, and in so doing would have actually fulfilled the edict that "we don't break user space" instead
of violating it as you were doing, the metaphor seems entirely appropriate.
Quoted 2 lines
And the fact that the reverted patch broke the user space behavior before, didn't bother you?
Breakage how? If you mean that the fact that it broke the behavior pattern you expected, that breakage doesn't really exist until your patches are accepted, so it's not really broke right now, it just isn't what you want.
Quoted 32 lines
Quoted 23 lines
Quoted 2 lines
especially after your silence.
I want to address this separate from everything else.
With as much hostility and accusations as I saw fly on the original submission, several things happened:
1) The submission got bumped to the bottom of my queue because settling
squables is not the sort of thing maintainers want to deal with.
2) The submission got bumped to the bottom of my queue so you would have a chance to work things out and come to a mutually agreed solution. Such resolution is in the best interest of both the community and the technical correctness of the fix as, generally, an
agreed upon fix will have had more community input and refinement prior
to being accepted.
3) The submission will get put on hold for a short while regardless of
points 1 and 2 just so you have a moment to settle down, rethink thingsQuoted 8 lines
a little bit, and decide if you really want to pull that "put the maintainer in this position" trigger. Have you really examined all the
possible solutions? Is there really no superior alternative? Is there
really no way to come to agreement with the other community member(s)
you are disagreeing with?
Quoted 4 lines
And did you go to the mailing list and write something like that "the series on hold till solution will be found", like it was with hfi1- vnic
and ipoib?
No, and if you recall, there was a decent period of silence on that before I told you guys to work it out too.
Quoted 16 lines
The Answer is no, you didn't do it, you silently ignored discussion, understood
that the series is stuck and continued down the road.Quoted 11 lines
In the end, there will always be those things that a maintainer will
simply need to make a call on because there won't be a clear right or
wrong side and it just becomes a judgment call. But I don't think this
one even came close to being that unclear, so again I'm confused by your insistence on going this route instead of just working things out
with Chien.
Quoted 4 lines
Because, I truly believe that they proposed the nasty hack, which doesn't fix the real problem - inability to deal with losses of netlink
messages.
In so much as the iwpmd needs to be modified to do a resync, I agree with you. But, the fix they made caused the system to be "reliable enough" that the problem more or less disappeared. It is true that, given enough load, the problem could resurface, but that hasn't happened yet in the real world.
However, iwpmd is part of rdma-core now, so *anyone* can fix it to do the resyncs as needed (and I don't know, maybe it does, I just did a quick grep for ENOBUFS to see if they capture that error and do a resync on it, and saw no hits for that with grep).
Quoted 8 lines
And I realized it after I started to work on RDMA netlink and read RFC
and design documents and assumptions about netlink.In my opinion, it is one of the small examples why RDMA subsystem is so
hated. It is full of "custom" solutions to known problems and many of those "custom" solutions are mix of hacks and band aids.
I don't believe this solution was as custom as you say it is. The netlink core supports it and we aren't the only consumer. That makes it a bit more generic than you paint it here IMO.
Quoted 15 lines
There is no need to reply on it.
It is my personal opinion and my view of the RDMA subsystem as a newcomer.I'm OOO till Monday.
Happy weekends.Quoted 8 lines
--
Doug Ledford <[email protected]>GPG KeyID: B826A3330E572FDD Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E572FDD
Signature · 8 lines
Doug Ledford <[email protected]> GPG KeyID: B826A3330E572FDD Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD -- 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): 1501762973.117042.7.camel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"
Quoted 13 lines · Doug Ledford
Quoted 5 lines · Leon Romanovsky
Because, I truly believe that they proposed the nasty hack, which doesn't fix the real problem - inability to deal with losses of netlink
messages.In so much as the iwpmd needs to be modified to do a resync, I agree with you. But, the fix they made caused the system to be "reliable enough" that the problem more or less disappeared. It is true that, given enough load, the problem could resurface, but that hasn't happened yet in the real world.
Quoted 4 lines
However, iwpmd is part of rdma-core now, so *anyone* can fix it to do the resyncs as needed (and I don't know, maybe it does, I just did a quick grep for ENOBUFS to see if they capture that error and do a resync on it, and saw no hits for that with grep).
And in case this didn't sink in from the above statement, allow me to make it explicit:
Does it not strike you how much of a failure it was on your part to submit a kernel patch that breaks iwpmd, code that is now part of rdma- core, FOR WHICH YOU ARE COMAINTAINER, without the corresponding patches to rdma-core to fix the real issue? You don't get to tell Chien to "go fix his software" because his software is part of rdma-core for which you are co-maintainer and ultimately you bear responsibility for rdma- core as a whole whether Chien fixes his stuff or not! Your job is to make sure that rdma-core is never broken in the way you were going to break it. You had a conflict of interest here between your kernel work and your user space responsibilities and you did not choose well how to resolve that conflict.
Signature · 8 lines
Doug Ledford <[email protected]> GPG KeyID: B826A3330E572FDD Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD -- 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): 1501764159.117042.9.camel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"
Quoted 22 lines · Doug Ledford
Quoted 13 lines · Doug Ledford
Quoted 5 lines · Leon Romanovsky
Because, I truly believe that they proposed the nasty hack, which doesn't fix the real problem - inability to deal with losses of netlink
messages.In so much as the iwpmd needs to be modified to do a resync, I agree with you. But, the fix they made caused the system to be "reliable enough" that the problem more or less disappeared. It is true that, given enough load, the problem could resurface, but that hasn't happened yet in the real world.
Quoted 4 lines
However, iwpmd is part of rdma-core now, so *anyone* can fix it to do the resyncs as needed (and I don't know, maybe it does, I just did a quick grep for ENOBUFS to see if they capture that error and do a resync on it, and saw no hits for that with grep).
And in case this didn't sink in from the above statement, allow me to make it explicit:
Quoted 11 lines
Does it not strike you how much of a failure it was on your part to submit a kernel patch that breaks iwpmd, code that is now part of rdma- core, FOR WHICH YOU ARE COMAINTAINER, without the corresponding patches to rdma-core to fix the real issue? You don't get to tell Chien to "go fix his software" because his software is part of rdma-core for which you are co-maintainer and ultimately you bear responsibility for rdma- core as a whole whether Chien fixes his stuff or not! Your job is to make sure that rdma-core is never broken in the way you were going to break it. You had a conflict of interest here between your kernel work and your user space responsibilities and you did not choose well how to resolve that conflict.
Ohh, my god.
Doug,
There is no ANY breakage of UAPI and this was used as an argument without ANY supportive claim behind of it. Just for the fun, I used the same claim, but it looks like I was supposed to cut it once it was claimed and not silently skip.
The iwpmd in rdma-core didn't get ANY patches BEFORE the patch in question and AFTER.
So yes, I do responsible for the rdma-core and I'm enforcing the same policy as we are using kernel - push companies to fix the root cause and invest in the common infrastructure.
Chien is supported by huge corporation with many developers working on iWARP stack and it seems very natural to demand from them the right solution.
Publicly, I tried it twice for the rdma-core:
1. One company stepped extra mile and improved the infrastructure.
2. Another company vanished into the dark and their supper important feature wasn't so important at the end.
Thanks
Quoted 6 lines
--
Doug Ledford <[email protected]>GPG KeyID: B826A3330E572FDD Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD
A message that is not in the archive (sent off-list or not yet received): 20170806074751.GA3636-U/[email protected]
Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"
T24gU3VuLCAyMDE3LTA4LTA2IGF0IDEwOjQ3ICswMzAwLCBMZW9uIFJvbWFub3Zza3kgd3JvdGU6 DQo+IENoaWVuIGlzIHN1cHBvcnRlZCBieSBodWdlIGNvcnBvcmF0aW9uIHdpdGggbWFueSBkZXZl bG9wZXJzIHdvcmtpbmcgb24gaVdBUlAgc3RhY2sNCj4gYW5kIGl0IHNlZW1zIHZlcnkgbmF0dXJh bCB0byBkZW1hbmQgZnJvbSB0aGVtIHRoZSByaWdodCBzb2x1dGlvbi4NCg0KTGV0J3Mgc2VlIHdo ZXRoZXIgb3Igbm90IHRoZSBpbnRlcmZhY2UgYmV0d2VlbiBrZXJuZWwgYW5kIGl3cG1kIGdldHMg Zml4ZWQNCmJ5IHNvbWVvbmUgd2hvIHJlYWxseSBjYXJlcyBhYm91dCBpV0FSUC4gSWYgdGhpcyBp bnRlcmZhY2UgZG9lcyBub3QgZ2V0IGZpeGVkDQptYXliZSB0aGF0IHRlbGxzIHVzIHNvbWV0aGlu ZyBhYm91dCB0aGUgZWNvbm9taWMgaW1wb3J0YW5jZSB0b2RheSBvZiBpV0FSUA0KYWRhcHRlcnMg Li4uDQoNCkJhcnQuDQo=
--
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
PATCH 2/33 RDMA/netlink: Remove netlink clients infrastructure
In reply to a message not in the archiveFrom: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
RDMA netlink has complicated infrastructure to add and remove netlink clients to NETLINK_RDMA family. This complicates the code and not in use because not many clients are available (3 clients) and most of them (2 clients) are statically compiled together with netlink.c.
The following patch refactors RDMA netlink and opens door for the future patches which will be able to get rid of a lot of dead iwcm* code.
--- drivers/infiniband/core/cma.c | 6 +- drivers/infiniband/core/core_priv.h | 4 +- drivers/infiniband/core/device.c | 45 +++------ drivers/infiniband/core/iwcm.c | 10 +- drivers/infiniband/core/netlink.c | 185 +++++++++++++++++------------------- include/rdma/rdma_netlink.h | 13 +-- 6 files changed, 112 insertions(+), 151 deletions(-)
drivers/infiniband/core/cma.c +2 -4
@@ -4512,9 +4512,7 @@ static int __init cma_init(void)if (ret) goto err; -if (ibnl_add_client (RDMA_NL_RDMA_CM ,ARRAY_SIZE (cma_cb_table), - cma_cb_table)) -pr_warn ("RDMA CMA: failed to add netlink callback\n" ); +rdma_nl_register (RDMA_NL_RDMA_CM , cma_cb_table);cma_configfs_init ();return 0 ;
@@ -4531,7 +4529,7 @@ static int __init cma_init(void) static void __exit cma_cleanup(void) {cma_configfs_exit (); -ibnl_remove_client (RDMA_NL_RDMA_CM ); +rdma_nl_unregister (RDMA_NL_RDMA_CM );ib_unregister_client (&cma_client);unregister_netdevice_notifier (&cma_nb);rdma_addr_unregister_client (&addr_client);
drivers/infiniband/core/core_priv.h +2 -2
@@ -179,8 +179,8 @@ void ib_mad_cleanup(void);int ib_sa_init (void );void ib_sa_cleanup (void ); -int ibnl_init (void ); -void ibnl_cleanup (void ); +int rdma_nl_init (void ); +void rdma_nl_exit (void );/** * Check if there are any listeners to the netlink group
drivers/infiniband/core/device.c +12 -33
94 lines not on this page · raw message
drivers/infiniband/core/iwcm.c +3 -7
@@ -1175,12 +1175,8 @@ static int __init iw_cm_init(void) ret =iwpm_init (RDMA_NL_IWCM );if (ret)pr_err ("iw_cm: couldn't init iwpm\n" ); - - ret =ibnl_add_client (RDMA_NL_IWCM ,ARRAY_SIZE (iwcm_nl_cb_table), - iwcm_nl_cb_table); -if (ret) -pr_err ("iw_cm: couldn't register netlink callbacks\n" ); - +else +rdma_nl_register (RDMA_NL_IWCM , iwcm_nl_cb_table); iwcm_wq =alloc_ordered_workqueue ("iw_cm_wq" ,WQ_MEM_RECLAIM );if (!iwcm_wq)return -ENOMEM ;
@@ -1200,7 +1196,7 @@ static void __exit iw_cm_cleanup(void) {unregister_net_sysctl_table (iwcm_ctl_table_hdr);destroy_workqueue (iwcm_wq); -ibnl_remove_client (RDMA_NL_IWCM ); +rdma_nl_unregister (RDMA_NL_IWCM );iwpm_exit (RDMA_NL_IWCM ); }
drivers/infiniband/core/netlink.c +89 -96
249 lines not on this page · raw message
include/rdma/rdma_netlink.h +4 -9
@@ -11,23 +11,18 @@ struct ibnl_client_cbs { };/** -* Add a a client to the list of IB netlink exporters. +* Register client in RDMA netlink. * @index: Index of the added client -* @nops: Number of supported ops by the added client. * @cb_table: A table for op->callback -* -* Returns 0 on success or a negative error code. */ -int ibnl_add_client (int index,int nops, -const struct ibnl_client_cbs cb_table[]); +void rdma_nl_register (unsigned int index, +const struct ibnl_client_cbs cb_table[]);/** * Remove a client from IB netlink. * @index: Index of the removed IB client. -* -* Returns 0 on success or a negative error code. */ -int ibnl_remove_client (int index); +void rdma_nl_unregister (unsigned int index);/** * Put a new message in a supplied skb.
Signature · 6 lines
2.13.3 -- 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
PATCH 3/33 RDMA/netlink: Remove redundant owner option for netlink callbacks
In reply to a message not in the archiveFrom: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
Owner field is not needed to be set because netlink is part of ib_core which will be unloaded last after all other modules are unloaded.
--- drivers/infiniband/core/cma.c | 3 +-- drivers/infiniband/core/netlink.c | 2 -- include/rdma/rdma_netlink.h | 1 - 3 files changed, 1 insertion(+), 5 deletions(-)
drivers/infiniband/core/cma.c +1 -2
@@ -4459,8 +4459,7 @@ static int cma_get_id_stats(struct sk_buff *skb, struct netlink_callback *cb) }static const struct ibnl_client_cbs cma_cb_table[] = { - [RDMA_NL_RDMA_CM_ID_STATS ] = { .dump = cma_get_id_stats, - .module =THIS_MODULE }, + [RDMA_NL_RDMA_CM_ID_STATS ] = { .dump = cma_get_id_stats}, };static int cma_init_net (struct net *net)
drivers/infiniband/core/netlink.c +0 -2
@@ -180,12 +180,10 @@ static int ibnl_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh, cb.skb = skb; cb.nlh = nlh; cb.dump = rdma_nl_types[index].cb_table[op].dump; - cb.module = rdma_nl_types[index].cb_table[op].module;return cb.dump (skb, &cb); } c.dump = rdma_nl_types[index].cb_table[op].dump; - c.module = rdma_nl_types[index].cb_table[op].module;return netlink_dump_start (nls, skb, nlh, &c); }
include/rdma/rdma_netlink.h +0 -1
@@ -7,7 +7,6 @@struct ibnl_client_cbs {int (*dump)(struct sk_buff *skb,struct netlink_callback *nlcb); -struct module *module; };/**
Signature · 6 lines
2.13.3 -- 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
PATCH 4/33 RDMA/netlink: Avoid double pass for RDMA netlink messages
In reply to a message not in the archiveFrom: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
The standard netlink_rcv_skb function skips messages without NLM_F_REQUEST flag in it, while SA netlink client issues them.
In commit bc10ed7d3d19 ("IB/core: Add rdma netlink helper functions") the local function was introduced to allow such messages.
This led to double pass for every incoming message.
In this patch, we unify that local implementation and netlink_rcv_skb functions, so there will be no need for double pass anymore.
As a outcome, this combined function gained more strict check for NLM_F_REQUEST flag and it is now allowed for SA pathquery client only.
--- drivers/infiniband/core/netlink.c | 62 +++++++++++++++++++++++++++------------ 1 file changed, 44 insertions(+), 18 deletions(-)
drivers/infiniband/core/netlink.c +44 -18
@@ -159,8 +159,8 @@ int ibnl_put_attr(struct sk_buff *skb, struct nlmsghdr *nlh, }EXPORT_SYMBOL (ibnl_put_attr); -static int ibnl_rcv_msg (struct sk_buff *skb,struct nlmsghdr *nlh, -struct netlink_ext_ack *extack) +static int rdma_nl_rcv_msg (struct sk_buff *skb,struct nlmsghdr *nlh, +struct netlink_ext_ack *extack) {int type = nlh->nlmsg_type;unsigned int index =RDMA_NL_GET_CLIENT (type);
@@ -187,40 +187,66 @@ static int ibnl_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh,return netlink_dump_start (nls, skb, nlh, &c); } -static void ibnl_rcv_reply_skb (struct sk_buff *skb) +/* +* This function is similar to netlink_rcv_skb with one exception: +* It calls to the callback for the netlink messages without NLM_F_REQUEST +* flag. These messages are intended for RDMA_NL_LS consumer, so it is allowed +* for that consumer only. +*/ +static int rdma_nl_rcv_skb (struct sk_buff *skb,int (*cb)(struct sk_buff *, +struct nlmsghdr *, +struct netlink_ext_ack *)) { +struct netlink_ext_ack extack = {};struct nlmsghdr *nlh; -int msglen; +int err; -/* -* Process responses until there is no more message or the first -* request. Generally speaking, it is not recommended to mix responses -* with requests. -*/ while (skb->len >=nlmsg_total_size (0 )) { +int msglen; + nlh =nlmsg_hdr (skb); + err =0 ;if (nlh->nlmsg_len <NLMSG_HDRLEN || skb->len < nlh->nlmsg_len) -return ; +return 0 ; -/* Handle response only */ -if (nlh->nlmsg_flags &NLM_F_REQUEST ) -return ; +/* +* Generally speaking, the only requests are handled +* by the kernel, but RDMA_NL_LS is different, because it +* runs backward netlink scheme. Kernel initiates messages +* and waits for reply with data to keep pathrecord cache +* in sync. +*/ +if (!(nlh->nlmsg_flags &NLM_F_REQUEST ) && + (RDMA_NL_GET_CLIENT (nlh->nlmsg_type) !=RDMA_NL_LS )) + goto ack; + +/* Skip control messages */ +if (nlh->nlmsg_type <NLMSG_MIN_TYPE ) + goto ack; + + err =cb (skb, nlh, &extack); +if (err == -EINTR ) + goto skip; -ibnl_rcv_msg (skb, nlh,NULL ); +ack: +if (nlh->nlmsg_flags &NLM_F_ACK || err) +netlink_ack (skb, nlh, err, &extack); +skip: msglen =NLMSG_ALIGN (nlh->nlmsg_len);if (msglen > skb->len) msglen = skb->len;skb_pull (skb, msglen); } + +return 0 ; } -static void ibnl_rcv (struct sk_buff *skb) +static void rdma_nl_rcv (struct sk_buff *skb) {mutex_lock (&rdma_nl_mutex); -ibnl_rcv_reply_skb (skb); -netlink_rcv_skb (skb, &ibnl_rcv_msg); +rdma_nl_rcv_skb (skb, &rdma_nl_rcv_msg);mutex_unlock (&rdma_nl_mutex); }
@@ -241,7 +267,7 @@ EXPORT_SYMBOL(ibnl_multicast);int __init rdma_nl_init (void ) {struct netlink_kernel_cfg cfg = { - .input = ibnl_rcv, + .input = rdma_nl_rcv, }; nls =netlink_kernel_create (&init_net,NETLINK_RDMA , &cfg);
Signature · 6 lines
2.13.3 -- 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
PATCH 5/33 RDMA/iwcm: Remove useless check of nelink client validity
In reply to a message not in the archiveFrom: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
RDMA netlink implementation guarantees that supplied client number is in allowed range.
--- drivers/infiniband/core/iwpm_util.c | 6 ------ 1 file changed, 6 deletions(-)
drivers/infiniband/core/iwpm_util.c +0 -6
@@ -54,8 +54,6 @@ static struct iwpm_admin_data iwpm_admin;int iwpm_init (u8 nl_client) {int ret =0 ; -if (iwpm_valid_client (nl_client)) -return -EINVAL ;mutex_lock (&iwpm_admin_lock);if (atomic_read (&iwpm_admin.refcount) ==0 ) { iwpm_hash_bucket = kzalloc(IWPM_MAPINFO_HASH_SIZE *
@@ -383,15 +381,11 @@ int iwpm_get_nlmsg_seq(void)int iwpm_valid_client (u8 nl_client) { -if (nl_client >=RDMA_NL_NUM_CLIENTS ) -return 0 ;return iwpm_admin.client_list[nl_client]; }void iwpm_set_valid (u8 nl_client,int valid) { -if (nl_client >=RDMA_NL_NUM_CLIENTS ) -return ; iwpm_admin.client_list[nl_client] = valid; }
Signature · 6 lines
2.13.3 -- 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
PATCH 6/33 RDMA/iwcm: Remove extra EXPORT_SYMBOLS
In reply to a message not in the archiveFrom: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
The iwcm exports functions which are not used outside of ib_core. This patch simply removes these EXPORT_SYMBOLS.
--- drivers/infiniband/core/iwpm_msg.c | 12 ------------ drivers/infiniband/core/iwpm_util.c | 5 ----- 2 files changed, 17 deletions(-)
drivers/infiniband/core/iwpm_msg.c +0 -12
@@ -42,7 +42,6 @@ int iwpm_valid_pid(void) {return iwpm_user_pid >0 ; } -EXPORT_SYMBOL (iwpm_valid_pid);/* * iwpm_register_pid - Send a netlink query to user space
@@ -122,7 +121,6 @@ int iwpm_register_pid(struct iwpm_dev_data *pm_msg, u8 nl_client)iwpm_free_nlmsg_request (&nlmsg_request->kref);return ret; } -EXPORT_SYMBOL (iwpm_register_pid);/* * iwpm_add_mapping - Send a netlink add mapping message
@@ -191,7 +189,6 @@ int iwpm_add_mapping(struct iwpm_sa_data *pm_msg, u8 nl_client)iwpm_free_nlmsg_request (&nlmsg_request->kref);return ret; } -EXPORT_SYMBOL (iwpm_add_mapping);/* * iwpm_add_and_query_mapping - Send a netlink add and query
@@ -267,7 +264,6 @@ int iwpm_add_and_query_mapping(struct iwpm_sa_data *pm_msg, u8 nl_client)iwpm_free_nlmsg_request (&nlmsg_request->kref);return ret; } -EXPORT_SYMBOL (iwpm_add_and_query_mapping);/* * iwpm_remove_mapping - Send a netlink remove mapping message
@@ -328,7 +324,6 @@ int iwpm_remove_mapping(struct sockaddr_storage *local_addr, u8 nl_client)dev_kfree_skb_any (skb);return ret; } -EXPORT_SYMBOL (iwpm_remove_mapping);/* netlink attribute policy for the received response to register pid request */ static const struct nla_policy resp_reg_policy[IWPM_NLA_RREG_PID_MAX ] = {
@@ -397,7 +392,6 @@ int iwpm_register_pid_cb(struct sk_buff *skb, struct netlink_callback *cb)up (&nlmsg_request->sem);return 0 ; } -EXPORT_SYMBOL (iwpm_register_pid_cb);/* netlink attribute policy for the received response to add mapping request */ static const struct nla_policy resp_add_policy[IWPM_NLA_RMANAGE_MAPPING_MAX ] = {
@@ -466,7 +460,6 @@ int iwpm_add_mapping_cb(struct sk_buff *skb, struct netlink_callback *cb)up (&nlmsg_request->sem);return 0 ; } -EXPORT_SYMBOL (iwpm_add_mapping_cb);/* netlink attribute policy for the response to add and query mapping request * and response with remote address info */
@@ -558,7 +551,6 @@ int iwpm_add_and_query_mapping_cb(struct sk_buff *skb,up (&nlmsg_request->sem);return 0 ; } -EXPORT_SYMBOL (iwpm_add_and_query_mapping_cb);/* * iwpm_remote_info_cb - Process a port mapper message, containing
@@ -627,7 +619,6 @@ int iwpm_remote_info_cb(struct sk_buff *skb, struct netlink_callback *cb) "remote_info: Mapped remote sockaddr:");return ret; } -EXPORT_SYMBOL (iwpm_remote_info_cb);/* netlink attribute policy for the received request for mapping info */ static const struct nla_policy resp_mapinfo_policy[IWPM_NLA_MAPINFO_REQ_MAX ] = {
@@ -677,7 +668,6 @@ int iwpm_mapping_info_cb(struct sk_buff *skb, struct netlink_callback *cb) ret =iwpm_send_mapinfo (nl_client, iwpm_user_pid);return ret; } -EXPORT_SYMBOL (iwpm_mapping_info_cb);/* netlink attribute policy for the received mapping info ack */ static const struct nla_policy ack_mapinfo_policy[IWPM_NLA_MAPINFO_NUM_MAX ] = {
@@ -707,7 +697,6 @@ int iwpm_ack_mapping_info_cb(struct sk_buff *skb, struct netlink_callback *cb)atomic_set (&echo_nlmsg_seq, cb->nlh->nlmsg_seq);return 0 ; } -EXPORT_SYMBOL (iwpm_ack_mapping_info_cb);/* netlink attribute policy for the received port mapper error message */ static const struct nla_policy map_error_policy[IWPM_NLA_ERR_MAX ] = {
@@ -751,4 +740,3 @@ int iwpm_mapping_error_cb(struct sk_buff *skb, struct netlink_callback *cb)up (&nlmsg_request->sem);return 0 ; } -EXPORT_SYMBOL (iwpm_mapping_error_cb);
drivers/infiniband/core/iwpm_util.c +0 -5
@@ -81,7 +81,6 @@ int iwpm_init(u8 nl_client) }return ret; } -EXPORT_SYMBOL (iwpm_init);static void free_hash_bucket (void );static void free_reminfo_bucket (void );
@@ -107,7 +106,6 @@ int iwpm_exit(u8 nl_client)iwpm_set_registration (nl_client,IWPM_REG_UNDEF );return 0 ; } -EXPORT_SYMBOL (iwpm_exit);static struct hlist_head *get_mapinfo_hash_bucket (struct sockaddr_storage *,struct sockaddr_storage *);
@@ -146,7 +144,6 @@ int iwpm_create_mapinfo(struct sockaddr_storage *local_sockaddr,spin_unlock_irqrestore (&iwpm_mapinfo_lock, flags);return ret; } -EXPORT_SYMBOL (iwpm_create_mapinfo);int iwpm_remove_mapinfo (struct sockaddr_storage *local_sockaddr,struct sockaddr_storage *mapped_local_addr)
@@ -182,7 +179,6 @@ int iwpm_remove_mapinfo(struct sockaddr_storage *local_sockaddr,spin_unlock_irqrestore (&iwpm_mapinfo_lock, flags);return ret; } -EXPORT_SYMBOL (iwpm_remove_mapinfo);static void free_hash_bucket (void ) {
@@ -295,7 +291,6 @@ int iwpm_get_remote_info(struct sockaddr_storage *mapped_loc_addr,spin_unlock_irqrestore (&iwpm_reminfo_lock, flags);return ret; } -EXPORT_SYMBOL (iwpm_get_remote_info);struct iwpm_nlmsg_request *iwpm_get_nlmsg_request (__u32 nlmsg_seq,u8 nl_client,gfp_t gfp)
Signature · 6 lines
2.13.3 -- 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
PATCH 7/33 RDMA/netlink: Add flag to consolidate common handing
In reply to a message not in the archiveFrom: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
Add ability to provide flags to control RDMA netlink callbacks and convert addr.c and sa_query.c to be first users of such infrastructure. It allows to move their CAP_NET_ADMIN checks into netlink core.
--- drivers/infiniband/core/addr.c | 3 +-- drivers/infiniband/core/device.c | 12 +++++++++--- drivers/infiniband/core/netlink.c | 4 ++++ drivers/infiniband/core/sa_query.c | 6 ++---- include/rdma/rdma_netlink.h | 6 ++++++ 5 files changed, 22 insertions(+), 9 deletions(-)
drivers/infiniband/core/addr.c +1 -2
@@ -134,8 +134,7 @@ int ib_nl_handle_ip_res_resp(struct sk_buff *skb,const struct nlmsghdr *nlh = (struct nlmsghdr *)cb->nlh;if ((nlh->nlmsg_flags &NLM_F_REQUEST ) || - !(NETLINK_CB (skb).sk) || - !netlink_capable (skb,CAP_NET_ADMIN )) + !(NETLINK_CB (skb).sk))return -EPERM ;if (ib_nl_is_good_ip_resp (nlh))
drivers/infiniband/core/device.c +9 -3
@@ -1088,11 +1088,17 @@ EXPORT_SYMBOL(ib_get_net_dev_by_params);static const struct ibnl_client_cbs ibnl_ls_cb_table[] = { [RDMA_NL_LS_OP_RESOLVE ] = { - .dump = ib_nl_handle_resolve_resp}, + .dump = ib_nl_handle_resolve_resp, + .flags =RDMA_NL_ADMIN_PERM , + }, [RDMA_NL_LS_OP_SET_TIMEOUT ] = { - .dump = ib_nl_handle_set_timeout}, + .dump = ib_nl_handle_set_timeout, + .flags =RDMA_NL_ADMIN_PERM , + }, [RDMA_NL_LS_OP_IP_RESOLVE ] = { - .dump = ib_nl_handle_ip_res_resp}, + .dump = ib_nl_handle_ip_res_resp, + .flags =RDMA_NL_ADMIN_PERM , + }, };static int __init ib_core_init (void )
drivers/infiniband/core/netlink.c +4 -0
@@ -171,6 +171,10 @@ static int rdma_nl_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh,if (!is_nl_valid (index, op))return -EINVAL ; +if ((rdma_nl_types[index].cb_table[op].flags &RDMA_NL_ADMIN_PERM ) && + !netlink_capable (skb,CAP_NET_ADMIN )) +return -EPERM ; +/* * For response or local service set_timeout request, * there is no need to use netlink_dump_start.
drivers/infiniband/core/sa_query.c +2 -4
@@ -1033,8 +1033,7 @@ int ib_nl_handle_set_timeout(struct sk_buff *skb,int ret;if (!(nlh->nlmsg_flags &NLM_F_REQUEST ) || - !(NETLINK_CB (skb).sk) || - !netlink_capable (skb,CAP_NET_ADMIN )) + !(NETLINK_CB (skb).sk))return -EPERM ; ret = nla_parse(tb, LS_NLA_TYPE_MAX - 1, nlmsg_data(nlh),
@@ -1109,8 +1108,7 @@ int ib_nl_handle_resolve_resp(struct sk_buff *skb,int ret;if ((nlh->nlmsg_flags &NLM_F_REQUEST ) || - !(NETLINK_CB (skb).sk) || - !netlink_capable (skb,CAP_NET_ADMIN )) + !(NETLINK_CB (skb).sk))return -EPERM ;spin_lock_irqsave (&ib_nl_request_lock, flags);
include/rdma/rdma_netlink.h +6 -0
@@ -7,6 +7,12 @@struct ibnl_client_cbs {int (*dump)(struct sk_buff *skb,struct netlink_callback *nlcb); +u8 flags; +}; + +enum rdma_nl_flags { +/* Require CAP_NET_ADMIN */ +RDMA_NL_ADMIN_PERM =1 <<0 , };/**
Signature · 6 lines
2.13.3 -- 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
PATCH 8/33 RDMA/netlink: Simplify the put_msg and put_attr
In reply to a message not in the archiveFrom: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
Reuse standard macros to cancel the netlink message in case of error.
--- drivers/infiniband/core/netlink.c | 31 +++++++++---------------------- 1 file changed, 9 insertions(+), 22 deletions(-)
drivers/infiniband/core/netlink.c +9 -22
@@ -126,36 +126,23 @@ EXPORT_SYMBOL(rdma_nl_unregister);void *ibnl_put_msg (struct sk_buff *skb,struct nlmsghdr **nlh,int seq,int len,int client,int op,int flags) { -unsigned char *prev_tail; - - prev_tail =skb_tail_pointer (skb); - *nlh =nlmsg_put (skb,0 , seq,RDMA_NL_GET_TYPE (client, op), - len, flags); -if (!*nlh) - goto out_nlmsg_trim; - (*nlh)->nlmsg_len =skb_tail_pointer (skb) - prev_tail; + *nlh =nlmsg_put (skb,0 , seq,RDMA_NL_GET_TYPE (client, op), len, flags); +if (!*nlh) { +nlmsg_cancel (skb, *nlh); +return NULL ; + }return nlmsg_data (*nlh); - -out_nlmsg_trim: -nlmsg_trim (skb, prev_tail); -return NULL ; }EXPORT_SYMBOL (ibnl_put_msg);int ibnl_put_attr (struct sk_buff *skb,struct nlmsghdr *nlh,int len,void *data,int type) { -unsigned char *prev_tail; - - prev_tail =skb_tail_pointer (skb); -if (nla_put (skb, type, len, data)) - goto nla_put_failure; - nlh->nlmsg_len +=skb_tail_pointer (skb) - prev_tail; +if (nla_put (skb, type, len, data)) { +nlmsg_cancel (skb, nlh); +return -EMSGSIZE ; + }return 0 ; - -nla_put_failure: -nlmsg_trim (skb, prev_tail - nlh->nlmsg_len); -return -EMSGSIZE ; }EXPORT_SYMBOL (ibnl_put_attr);
Signature · 6 lines
2.13.3 -- 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
PATCH 9/33 RDMA/netlink: Rename and remove redundant parameter from ibnl_unicast
In reply to a message not in the archiveFrom: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
Netlink message header is not needed for unicast reply, hence remove it.
--- drivers/infiniband/core/iwpm_msg.c | 6 +++--- drivers/infiniband/core/iwpm_util.c | 4 ++-- drivers/infiniband/core/netlink.c | 5 ++--- include/rdma/rdma_netlink.h | 4 +--- 4 files changed, 8 insertions(+), 11 deletions(-)
drivers/infiniband/core/iwpm_msg.c +3 -3
@@ -172,7 +172,7 @@ int iwpm_add_mapping(struct iwpm_sa_data *pm_msg, u8 nl_client) goto add_mapping_error; nlmsg_request->req_buffer = pm_msg; - ret =ibnl_unicast (skb, nlh, iwpm_user_pid); + ret =rdma_nl_unicast (skb, iwpm_user_pid);if (ret) { skb =NULL ;/* skb is freed in the netlink send-op handling */ iwpm_user_pid =IWPM_PID_UNDEFINED ;
@@ -248,7 +248,7 @@ int iwpm_add_and_query_mapping(struct iwpm_sa_data *pm_msg, u8 nl_client) goto query_mapping_error; nlmsg_request->req_buffer = pm_msg; - ret =ibnl_unicast (skb, nlh, iwpm_user_pid); + ret =rdma_nl_unicast (skb, iwpm_user_pid);if (ret) { skb =NULL ;/* skb is freed in the netlink send-op handling */ err_str ="Unable to send a nlmsg" ;
@@ -308,7 +308,7 @@ int iwpm_remove_mapping(struct sockaddr_storage *local_addr, u8 nl_client)if (ret) goto remove_mapping_error; - ret =ibnl_unicast (skb, nlh, iwpm_user_pid); + ret =rdma_nl_unicast (skb, iwpm_user_pid);if (ret) { skb =NULL ;/* skb is freed in the netlink send-op handling */ iwpm_user_pid =IWPM_PID_UNDEFINED ;
drivers/infiniband/core/iwpm_util.c +2 -2
@@ -597,7 +597,7 @@ static int send_mapinfo_num(u32 mapping_num, u8 nl_client, int iwpm_pid) &mapping_num, IWPM_NLA_MAPINFO_SEND_NUM);if (ret) goto mapinfo_num_error; - ret =ibnl_unicast (skb, nlh, iwpm_pid); + ret =rdma_nl_unicast (skb, iwpm_pid);if (ret) { skb =NULL ; err_str ="Unable to send a nlmsg" ;
@@ -626,7 +626,7 @@ static int send_nlmsg_done(struct sk_buff *skb, u8 nl_client, int iwpm_pid)return -ENOMEM ; } nlh->nlmsg_type =NLMSG_DONE ; - ret =ibnl_unicast (skb, (struct nlmsghdr *)skb->data, iwpm_pid); + ret =rdma_nl_unicast (skb, iwpm_pid);if (ret)pr_warn ("%s Unable to send a nlmsg\n" , __func__);return ret;
drivers/infiniband/core/netlink.c +2 -3
@@ -241,12 +241,11 @@ static void rdma_nl_rcv(struct sk_buff *skb)mutex_unlock (&rdma_nl_mutex); } -int ibnl_unicast (struct sk_buff *skb,struct nlmsghdr *nlh, -__u32 pid) +int rdma_nl_unicast (struct sk_buff *skb,u32 pid) {return nlmsg_unicast (nls, skb, pid); } -EXPORT_SYMBOL (ibnl_unicast); +EXPORT_SYMBOL (rdma_nl_unicast);int ibnl_multicast (struct sk_buff *skb,struct nlmsghdr *nlh,unsigned int group,gfp_t flags)
include/rdma/rdma_netlink.h +1 -3
@@ -56,12 +56,10 @@ int ibnl_put_attr(struct sk_buff *skb, struct nlmsghdr *nlh,/** * Send the supplied skb to a specific userspace PID. * @skb: The netlink skb -* @nlh: Header of the netlink message to send * @pid: Userspace netlink process ID * Returns 0 on success or a negative error code. */ -int ibnl_unicast (struct sk_buff *skb,struct nlmsghdr *nlh, -__u32 pid); +int rdma_nl_unicast (struct sk_buff *skb,u32 pid);/** * Send the supplied skb to a netlink group.
Signature · 6 lines
2.13.3 -- 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
PATCH 10/33 RDMA/netlink: Rename and remove redundant parameter from ibnl_multicast
In reply to a message not in the archiveFrom: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
The pointer to netlink header was not used in the ibnl_multicast function, so let's remove it and simplify the function signature.
--- drivers/infiniband/core/addr.c | 2 +- drivers/infiniband/core/iwpm_msg.c | 2 +- drivers/infiniband/core/netlink.c | 5 ++--- drivers/infiniband/core/sa_query.c | 2 +- include/rdma/rdma_netlink.h | 4 +--- 5 files changed, 6 insertions(+), 9 deletions(-)
drivers/infiniband/core/addr.c +1 -1
@@ -184,7 +184,7 @@ static int ib_nl_ip_send_msg(struct rdma_dev_addr *dev_addr,/* Repair the nlmsg header length */ nlmsg_end (skb, nlh); -ibnl_multicast (skb, nlh,RDMA_NL_GROUP_LS ,GFP_KERNEL ); +rdma_nl_multicast (skb,RDMA_NL_GROUP_LS ,GFP_KERNEL );/* Make the request retry, so when we get the response from userspace * we will have something.
drivers/infiniband/core/iwpm_msg.c +1 -1
@@ -103,7 +103,7 @@ int iwpm_register_pid(struct iwpm_dev_data *pm_msg, u8 nl_client)pr_debug ("%s: Multicasting a nlmsg (dev = %s ifname = %s iwpm = %s)\n" , __func__, pm_msg->dev_name, pm_msg->if_name, iwpm_ulib_name); - ret =ibnl_multicast (skb, nlh,RDMA_NL_GROUP_IWPM ,GFP_KERNEL ); + ret =rdma_nl_multicast (skb,RDMA_NL_GROUP_IWPM ,GFP_KERNEL );if (ret) { skb =NULL ;/* skb is freed in the netlink send-op handling */ iwpm_user_pid =IWPM_PID_UNAVAILABLE ;
drivers/infiniband/core/netlink.c +2 -3
@@ -247,12 +247,11 @@ int rdma_nl_unicast(struct sk_buff *skb, u32 pid) }EXPORT_SYMBOL (rdma_nl_unicast); -int ibnl_multicast (struct sk_buff *skb,struct nlmsghdr *nlh, -unsigned int group,gfp_t flags) +int rdma_nl_multicast (struct sk_buff *skb,unsigned int group,gfp_t flags) {return nlmsg_multicast (nls, skb,0 , group, flags); } -EXPORT_SYMBOL (ibnl_multicast); +EXPORT_SYMBOL (rdma_nl_multicast);int __init rdma_nl_init (void ) {
drivers/infiniband/core/sa_query.c +1 -1
@@ -861,7 +861,7 @@ static int ib_nl_send_msg(struct ib_sa_query *query, gfp_t gfp_mask)/* Repair the nlmsg header length */ nlmsg_end (skb, nlh); - ret =ibnl_multicast (skb, nlh,RDMA_NL_GROUP_LS , gfp_mask); + ret =rdma_nl_multicast (skb,RDMA_NL_GROUP_LS , gfp_mask);if (!ret) ret = len;else
include/rdma/rdma_netlink.h +1 -3
@@ -64,12 +64,10 @@ int rdma_nl_unicast(struct sk_buff *skb, u32 pid);/** * Send the supplied skb to a netlink group. * @skb: The netlink skb -* @nlh: Header of the netlink message to send * @group: Netlink group ID * @flags: allocation flags * Returns 0 on success or a negative error code. */ -int ibnl_multicast (struct sk_buff *skb,struct nlmsghdr *nlh, -unsigned int group,gfp_t flags); +int rdma_nl_multicast (struct sk_buff *skb,unsigned int group,gfp_t flags);#endif /* _RDMA_NETLINK_H */
Signature · 6 lines
2.13.3 -- 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
PATCH 11/33 RDMA/netlink: Simplify and rename ibnl_chk_listeners
In reply to a message not in the archiveFrom: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
Make ibnl_chk_listeners function to be one line by removing unneeded comparison.
Rename that function to be complaint to other functions in RDMA netlink.
--- drivers/infiniband/core/addr.c | 2 +- drivers/infiniband/core/netlink.c | 7 +++---- drivers/infiniband/core/sa_query.c | 2 +- include/rdma/rdma_netlink.h | 6 ++++++ 4 files changed, 11 insertions(+), 6 deletions(-)
drivers/infiniband/core/addr.c +1 -1
@@ -325,7 +325,7 @@ static void queue_req(struct addr_req *req)static int ib_nl_fetch_ha (struct dst_entry *dst,struct rdma_dev_addr *dev_addr,const void *daddr,u32 seq,u16 family) { -if (ibnl_chk_listeners (RDMA_NL_GROUP_LS )) +if (rdma_nl_chk_listeners (RDMA_NL_GROUP_LS ))return -EADDRNOTAVAIL ;/* We fill in what we can, the response will fill the rest */
drivers/infiniband/core/netlink.c +3 -4
@@ -47,12 +47,11 @@ static struct {const struct ibnl_client_cbs *cb_table; } rdma_nl_types[RDMA_NL_NUM_CLIENTS ]; -int ibnl_chk_listeners (unsigned int group) +int rdma_nl_chk_listeners (unsigned int group) { -if (netlink_has_listeners (nls, group) ==0 ) -return -1 ; -return 0 ; +return (netlink_has_listeners (nls, group)) ?0 :-1 ; } +EXPORT_SYMBOL (rdma_nl_chk_listeners);static bool is_nl_msg_valid (unsigned int type,unsigned int op) {
drivers/infiniband/core/sa_query.c +1 -1
@@ -1418,7 +1418,7 @@ static int send_mad(struct ib_sa_query *query, int timeout_ms, gfp_t gfp_mask)if ((query->flags &IB_SA_ENABLE_LOCAL_SERVICE ) && (!(query->flags &IB_SA_QUERY_OPA ))) { -if (!ibnl_chk_listeners (RDMA_NL_GROUP_LS )) { +if (!rdma_nl_chk_listeners (RDMA_NL_GROUP_LS )) {if (!ib_nl_make_request (query, gfp_mask))return id; }
include/rdma/rdma_netlink.h +6 -0
@@ -70,4 +70,10 @@ int rdma_nl_unicast(struct sk_buff *skb, u32 pid);*/ int rdma_nl_multicast (struct sk_buff *skb,unsigned int group,gfp_t flags); +/** +* Check if there are any listeners to the netlink group +* @group: the netlink group ID +* Returns 0 on success or a negative for no listeners. +*/ +int rdma_nl_chk_listeners (unsigned int group);#endif /* _RDMA_NETLINK_H */
Signature · 6 lines
2.13.3 -- 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
PATCH 12/33 RDMA/netlink: Rename netlink callback struct
In reply to a message not in the archiveFrom: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
The RDMA netlink client infrastructure was removed and made obsolete the old name (ibnl_client_cbs). This patch renames to the more appropriate name (rdma_nl_cbs).
--- drivers/infiniband/core/cma.c | 2 +- drivers/infiniband/core/device.c | 2 +- drivers/infiniband/core/iwcm.c | 2 +- drivers/infiniband/core/netlink.c | 4 ++-- include/rdma/rdma_netlink.h | 4 ++-- 5 files changed, 7 insertions(+), 7 deletions(-)
drivers/infiniband/core/cma.c +1 -1
@@ -4458,7 +4458,7 @@ static int cma_get_id_stats(struct sk_buff *skb, struct netlink_callback *cb)return skb->len; } -static const struct ibnl_client_cbs cma_cb_table[] = { +static const struct rdma_nl_cbs cma_cb_table[] = { [RDMA_NL_RDMA_CM_ID_STATS ] = { .dump = cma_get_id_stats}, };
drivers/infiniband/core/device.c +1 -1
@@ -1086,7 +1086,7 @@ struct net_device *ib_get_net_dev_by_params(struct ib_device *dev, }EXPORT_SYMBOL (ib_get_net_dev_by_params); -static const struct ibnl_client_cbs ibnl_ls_cb_table[] = { +static const struct rdma_nl_cbs ibnl_ls_cb_table[] = { [RDMA_NL_LS_OP_RESOLVE ] = { .dump = ib_nl_handle_resolve_resp, .flags =RDMA_NL_ADMIN_PERM ,
drivers/infiniband/core/iwcm.c +1 -1
@@ -80,7 +80,7 @@ const char *__attribute_const__ iwcm_reject_msg(int reason) }EXPORT_SYMBOL (iwcm_reject_msg); -static struct ibnl_client_cbs iwcm_nl_cb_table[] = { +static struct rdma_nl_cbs iwcm_nl_cb_table[] = { [RDMA_NL_IWPM_REG_PID ] = {.dump = iwpm_register_pid_cb}, [RDMA_NL_IWPM_ADD_MAPPING ] = {.dump = iwpm_add_mapping_cb}, [RDMA_NL_IWPM_QUERY_MAPPING ] = {.dump = iwpm_add_and_query_mapping_cb},
drivers/infiniband/core/netlink.c +2 -2
@@ -44,7 +44,7 @@static DEFINE_MUTEX (rdma_nl_mutex);static struct sock *nls;static struct { -const struct ibnl_client_cbs *cb_table; +const struct rdma_nl_cbs *cb_table; } rdma_nl_types[RDMA_NL_NUM_CLIENTS ];int rdma_nl_chk_listeners (unsigned int group)
@@ -84,7 +84,7 @@ static bool is_nl_valid(unsigned int type, unsigned int op) }void rdma_nl_register (unsigned int index, -const struct ibnl_client_cbs cb_table[]) +const struct rdma_nl_cbs cb_table[]) {mutex_lock (&rdma_nl_mutex);if (!is_nl_msg_valid (index,0 )) {
include/rdma/rdma_netlink.h +2 -2
@@ -5,7 +5,7 @@#include <linux/netlink.h> #include <uapi/rdma/rdma_netlink.h> -struct ibnl_client_cbs { +struct rdma_nl_cbs {int (*dump)(struct sk_buff *skb,struct netlink_callback *nlcb);u8 flags; };
@@ -21,7 +21,7 @@ enum rdma_nl_flags {* @cb_table: A table for op->callback */ void rdma_nl_register (unsigned int index, -const struct ibnl_client_cbs cb_table[]); +const struct rdma_nl_cbs cb_table[]);/** * Remove a client from IB netlink.
Signature · 6 lines
2.13.3 -- 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
PATCH 13/33 RDMA/core: Add iterator over ib_devices
In reply to a message not in the archiveFrom: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
The coming nldev needs to iterate over all IB devices in the system and in order to not expose the ib_devices list outside the devices.c, it is necessary to provide function iterator.
Current version is written explicitly for nldev callback to avoid over-engineering at this stage, but it can be easily extended for other types.
--- drivers/infiniband/core/core_priv.h | 8 ++++++++ drivers/infiniband/core/device.c | 25 +++++++++++++++++++++++++ 2 files changed, 33 insertions(+)
drivers/infiniband/core/core_priv.h +8 -0
@@ -102,6 +102,14 @@ void ib_enum_all_roce_netdevs(roce_netdev_filter filter,roce_netdev_callback cb, void *cookie); +typedef int (*nldev_callback )(struct ib_device *device, +struct sk_buff *skb, +struct netlink_callback *cb, +unsigned int idx); + +int ib_enum_all_devs (nldev_callback nldev_cb,struct sk_buff *skb, +struct netlink_callback *cb); +enum ib_cache_gid_default_mode {IB_CACHE_GID_DEFAULT_MODE_SET ,IB_CACHE_GID_DEFAULT_MODE_DELETE
drivers/infiniband/core/device.c +25 -0
@@ -893,6 +893,31 @@ void ib_enum_all_roce_netdevs(roce_netdev_filter filter, }/** +* ib_enum_all_devs - enumerate all ib_devices +* @cb: Callback to call for each found ib_device +* +* Enumerates all ib_devices and calls callback() on each device. +*/ +int ib_enum_all_devs (nldev_callback nldev_cb,struct sk_buff *skb, +struct netlink_callback *cb) +{ +struct ib_device *dev; +unsigned int idx =0 ; +int ret =0 ; + +down_read (&lists_rwsem); +list_for_each_entry (dev, &device_list, core_list) { + ret =nldev_cb (dev, skb, cb, idx); +if (ret) +break ; + idx++; + } + +up_read (&lists_rwsem); +return ret; +} + +/** * ib_query_pkey - Get P_Key table entry * @device:Device to query * @port_num:Port number to query
Signature · 6 lines
2.13.3 -- 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
PATCH 14/33 RDMA/core: Add and expose static device index
In reply to a message not in the archiveFrom: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
This patch adds static device index in similar fashion to already available in netdev world (struct net->ifindex).
In downstream patches, the RDMA nelink will use this idx-to-ib_device conversion, so as part of this commit, we are exposing the translation function to be visible for IB/core users.
--- drivers/infiniband/core/core_priv.h | 5 +++++ drivers/infiniband/core/device.c | 37 ++++++++++++++++++++++++++++++++++++- include/rdma/ib_verbs.h | 2 ++ 3 files changed, 43 insertions(+), 1 deletion(-)
drivers/infiniband/core/core_priv.h +5 -0
@@ -309,4 +309,9 @@ static inline int ib_mad_enforce_security(struct ib_mad_agent_private *map,return 0 ; }#endif + +struct ib_device *__ib_device_get_by_index (u32 ifindex); +/* RDMA device netlink */ +void nldev_init (void ); +void nldev_exit (void );#endif /* _CORE_PRIV_H */
drivers/infiniband/core/device.c +36 -1
@@ -134,6 +134,17 @@ static int ib_device_check_mandatory(struct ib_device *device)return 0 ; } +struct ib_device *__ib_device_get_by_index (u32 index) +{ +struct ib_device *device; + +list_for_each_entry (device, &device_list, core_list) +if (device->index == index) +return device; + +return NULL ; +} +static struct ib_device *__ib_device_get_by_name (const char *name) {struct ib_device *device;
@@ -145,7 +156,6 @@ static struct ib_device *__ib_device_get_by_name(const char *name)return NULL ; } -static int alloc_name (char *name) {unsigned long *inuse;
@@ -395,6 +405,30 @@ static int ib_security_change(struct notifier_block *nb, unsigned long event, }/** +* __dev_new_index - allocate an device index +* +* Returns a suitable unique value for a new device interface +* number. It assumes that there are less than 2^32-1 ib devices +* will be present in the system. +*/ +static u32 __dev_new_index (void ) +{ +/* +* The device index to allow stable naming. +* Similar to struct net -> ifindex. +*/ +static u32 index; + +for (;;) { +if (!(++index)) + index =1 ; + +if (!__ib_device_get_by_index (index)) +return index; + } +} + +/** * ib_register_device - Register an IB device with IB core * @device:Device to register *
@@ -492,6 +526,7 @@ int ib_register_device(struct ib_device *device,if (client->add && !add_client_context (device, client)) client->add (device); + device->index =__dev_new_index ();down_write (&lists_rwsem);list_add_tail (&device->core_list, &device_list);up_write (&lists_rwsem);
include/rdma/ib_verbs.h +2 -0
@@ -2298,6 +2298,8 @@ struct ib_device {struct rdmacg_device cg_device;#endif +u32 index; +/** * The following mandatory functions are used only at device * registration. Keep functions such as these at the end of this
Signature · 6 lines
2.13.3 -- 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
PATCH 15/33 RDMA/netlink: Add and implement doit netlink callback
In reply to a message not in the archiveFrom: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
The .doit callback is used by netlink core to differentiate between get and set operations. Common convention is to use that call for command operations like (SET, ADD, e.t.c.) and/or access without NLF_M_DUMP flag.
This commit adds proper declaration and implementation to RDMA netlink.
--- drivers/infiniband/core/netlink.c | 19 ++++++++++++++----- include/rdma/rdma_netlink.h | 2 ++ 2 files changed, 16 insertions(+), 5 deletions(-)
drivers/infiniband/core/netlink.c +14 -5
@@ -76,9 +76,13 @@ static bool is_nl_msg_valid(unsigned int type, unsigned int op)static bool is_nl_valid (unsigned int type,unsigned int op) { -if (!is_nl_msg_valid (type, op) || - !rdma_nl_types[type].cb_table || - !rdma_nl_types[type].cb_table[op].dump) +const struct rdma_nl_cbs *cb_table; + +if (!is_nl_msg_valid (type, op)) +return false; + + cb_table = rdma_nl_types[type].cb_table; +if (!cb_table || (!cb_table[op].dump && !cb_table[op].doit))return false;return true; }
@@ -153,6 +157,7 @@ static int rdma_nl_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh,unsigned int op =RDMA_NL_GET_OP (type);struct netlink_callback cb = {};struct netlink_dump_control c = {}; +int ret;if (!is_nl_valid (index, op))return -EINVAL ;
@@ -171,10 +176,14 @@ static int rdma_nl_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh, cb.nlh = nlh; cb.dump = rdma_nl_types[index].cb_table[op].dump;return cb.dump (skb, &cb); + }else { + c.dump = rdma_nl_types[index].cb_table[op].dump; +return netlink_dump_start (nls, skb, nlh, &c); } +if (rdma_nl_types[index].cb_table[op].doit) + ret = rdma_nl_types[index].cb_table[op].doit (skb, nlh, extack); +return ret; - c.dump = rdma_nl_types[index].cb_table[op].dump; -return netlink_dump_start (nls, skb, nlh, &c); }/*
include/rdma/rdma_netlink.h +2 -0
@@ -6,6 +6,8 @@#include <uapi/rdma/rdma_netlink.h> struct rdma_nl_cbs { +int (*doit)(struct sk_buff *skb,struct nlmsghdr *nlh, +struct netlink_ext_ack *extack);int (*dump)(struct sk_buff *skb,struct netlink_callback *nlcb);u8 flags; };
Signature · 6 lines
2.13.3 -- 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
PATCH 16/33 RDMA/netlink: Reduce indirection access to cb_table
In reply to a message not in the archiveFrom: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
Introduce intermediate variable to store access to fields of cb_table.
--- drivers/infiniband/core/netlink.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)
drivers/infiniband/core/netlink.c +8 -5
@@ -157,12 +157,15 @@ static int rdma_nl_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh,unsigned int op =RDMA_NL_GET_OP (type);struct netlink_callback cb = {};struct netlink_dump_control c = {}; +const struct rdma_nl_cbs *cb_table;int ret;if (!is_nl_valid (index, op))return -EINVAL ; -if ((rdma_nl_types[index].cb_table[op].flags &RDMA_NL_ADMIN_PERM ) && + cb_table = rdma_nl_types[type].cb_table; + +if ((cb_table[op].flags &RDMA_NL_ADMIN_PERM ) && !netlink_capable (skb,CAP_NET_ADMIN ))return -EPERM ;
@@ -174,14 +177,14 @@ static int rdma_nl_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh, (index == RDMA_NL_LS && op == RDMA_NL_LS_OP_SET_TIMEOUT)) { cb.skb = skb; cb.nlh = nlh; - cb.dump = rdma_nl_types[index].cb_table[op].dump; + cb.dump = cb_table[op].dump; return cb.dump(skb, &cb); } else { - c.dump = rdma_nl_types[index].cb_table[op].dump; + c.dump = cb_table[op].dump;return netlink_dump_start (nls, skb, nlh, &c); } -if (rdma_nl_types[index].cb_table[op].doit) - ret = rdma_nl_types[index].cb_table[op].doit (skb, nlh, extack); +if (cb_table[op].doit) + ret = cb_table[op].doit (skb, nlh, extack);return ret; }
Signature · 6 lines
2.13.3 -- 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
PATCH 17/33 RDMA/netlink: Convert LS to doit callback
In reply to a message not in the archiveFrom: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
RDMA_NL_LS protocol is actually does not dump anything, but sets data and it should be handled by doit callback.
This patch actually converts RDMA_NL_LS to doit callback, while preserving IWCM and RDMA_CM flows through netlink_dump_start().
--- drivers/infiniband/core/addr.c | 5 ++--- drivers/infiniband/core/core_priv.h | 9 ++++++--- drivers/infiniband/core/device.c | 6 +++--- drivers/infiniband/core/netlink.c | 28 ++++++++++------------------ drivers/infiniband/core/sa_query.c | 8 ++++---- 5 files changed, 25 insertions(+), 31 deletions(-)
drivers/infiniband/core/addr.c +2 -3
@@ -129,10 +129,9 @@ static void ib_nl_process_good_ip_rsep(const struct nlmsghdr *nlh) }int ib_nl_handle_ip_res_resp (struct sk_buff *skb, -struct netlink_callback *cb) +struct nlmsghdr *nlh, +struct netlink_ext_ack *extack) { -const struct nlmsghdr *nlh = (struct nlmsghdr *)cb->nlh; -if ((nlh->nlmsg_flags &NLM_F_REQUEST ) || !(NETLINK_CB (skb).sk))return -EPERM ;
drivers/infiniband/core/core_priv.h +6 -3
@@ -198,11 +198,14 @@ void rdma_nl_exit(void);int ibnl_chk_listeners (unsigned int group);int ib_nl_handle_resolve_resp (struct sk_buff *skb, -struct netlink_callback *cb); +struct nlmsghdr *nlh, +struct netlink_ext_ack *extack);int ib_nl_handle_set_timeout (struct sk_buff *skb, -struct netlink_callback *cb); +struct nlmsghdr *nlh, +struct netlink_ext_ack *extack);int ib_nl_handle_ip_res_resp (struct sk_buff *skb, -struct netlink_callback *cb); +struct nlmsghdr *nlh, +struct netlink_ext_ack *extack); int ib_get_cached_subnet_prefix(struct ib_device *device, u8 port_num,
drivers/infiniband/core/device.c +3 -3
@@ -1148,15 +1148,15 @@ EXPORT_SYMBOL(ib_get_net_dev_by_params);static const struct rdma_nl_cbs ibnl_ls_cb_table[] = { [RDMA_NL_LS_OP_RESOLVE ] = { - .dump = ib_nl_handle_resolve_resp, + .doit = ib_nl_handle_resolve_resp, .flags =RDMA_NL_ADMIN_PERM , }, [RDMA_NL_LS_OP_SET_TIMEOUT ] = { - .dump = ib_nl_handle_set_timeout, + .doit = ib_nl_handle_set_timeout, .flags =RDMA_NL_ADMIN_PERM , }, [RDMA_NL_LS_OP_IP_RESOLVE ] = { - .dump = ib_nl_handle_ip_res_resp, + .doit = ib_nl_handle_ip_res_resp, .flags =RDMA_NL_ADMIN_PERM , }, };
drivers/infiniband/core/netlink.c +10 -18
@@ -155,38 +155,30 @@ static int rdma_nl_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh,int type = nlh->nlmsg_type;unsigned int index =RDMA_NL_GET_CLIENT (type);unsigned int op =RDMA_NL_GET_OP (type); -struct netlink_callback cb = {}; -struct netlink_dump_control c = {};const struct rdma_nl_cbs *cb_table; -int ret;if (!is_nl_valid (index, op))return -EINVAL ; - cb_table = rdma_nl_types[type].cb_table; + cb_table = rdma_nl_types[index].cb_table;if ((cb_table[op].flags &RDMA_NL_ADMIN_PERM ) && !netlink_capable (skb,CAP_NET_ADMIN ))return -EPERM ; -/* -* For response or local service set_timeout request, -* there is no need to use netlink_dump_start. -*/ -if (!(nlh->nlmsg_flags &NLM_F_REQUEST ) || - (index ==RDMA_NL_LS && op ==RDMA_NL_LS_OP_SET_TIMEOUT )) { - cb.skb = skb; - cb.nlh = nlh; - cb.dump = cb_table[op].dump; -return cb.dump (skb, &cb); - }else { - c.dump = cb_table[op].dump; +/* FIXME: Convert IWCM to properly handle doit callbacks */ +if ((nlh->nlmsg_flags &NLM_F_DUMP ) || index ==RDMA_NL_RDMA_CM || + index ==RDMA_NL_IWCM ) { +struct netlink_dump_control c = { + .dump = cb_table[op].dump, + };return netlink_dump_start (nls, skb, nlh, &c); } +if (cb_table[op].doit) - ret = cb_table[op].doit (skb, nlh, extack); -return ret; +return cb_table[op].doit (skb, nlh, extack); +return 0 ; }/*
drivers/infiniband/core/sa_query.c +4 -4
@@ -1021,9 +1021,9 @@ static void ib_nl_request_timeout(struct work_struct *work) }int ib_nl_handle_set_timeout (struct sk_buff *skb, -struct netlink_callback *cb) +struct nlmsghdr *nlh, +struct netlink_ext_ack *extack) { -const struct nlmsghdr *nlh = (struct nlmsghdr *)cb->nlh;int timeout, delta, abs_delta;const struct nlattr *attr;unsigned long flags;
@@ -1097,9 +1097,9 @@ static inline int ib_nl_is_good_resolve_resp(const struct nlmsghdr *nlh) }int ib_nl_handle_resolve_resp (struct sk_buff *skb, -struct netlink_callback *cb) +struct nlmsghdr *nlh, +struct netlink_ext_ack *extack) { -const struct nlmsghdr *nlh = (struct nlmsghdr *)cb->nlh;unsigned long flags;struct ib_sa_query *query;struct ib_mad_send_buf *send_buf;
Signature · 6 lines
2.13.3 -- 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
PATCH 18/33 RDMA/netlink: Update copyright
In reply to a message not in the archiveFrom: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
Add Mellanox to the copyright header.
--- drivers/infiniband/core/netlink.c | 1 + 1 file changed, 1 insertion(+)
drivers/infiniband/core/netlink.c +1 -0
@@ -1,4 +1,5 @@/* +* Copyright (c) 2017 Mellanox Technologies Inc. All rights reserved. * Copyright (c) 2010 Voltaire Inc. All rights reserved. * * This software is available to you under a choice of one of two
Signature · 6 lines
2.13.3 -- 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
PATCH 19/33 RDMA/netlink: Add netlink device definitions to UAPI
In reply to a message not in the archiveFrom: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
Introduce new defines to rdma_netlink.h, so the RDMA configuration tool will be able to communicate with RDMA subsystem by using the shared defines.
The addition of new client (NLDEV) revealed the fact that we exposed by mistake the RDMA_NL_I40IW define which is not backed by any RDMA netlink by now and it won't be exposed in the future too. So this patch reuses the value and deletes the old defines.
The NLDEV operates with objects. The struct ib_device has two straightforward
objects: device itself and ports of that device.
This brings us to propose the following commands to work on those objects:
* RDMA_NLDEV_CMD_{GET,SET,NEW,DEL} - works on ib_device itself
* RDMA_NLDEV_CMD_PORT_{GET,SET,NEW,DEL} - works on ports of specific ib_device
Those commands receive/return the device index (RDMA_NLDEV_ATTR_DEV_INDEX) and port index (RDMA_NLDEV_ATTR_PORT_INDEX). For device object accesses, the RDMA_NLDEV_ATTR_PORT_INDEX will return the maximum number of ports for specific ib_device and for port access the actual port index.
The port index starts from 1 to follow RDMA/core internal semantics and the sysfs exposed knobs.
--- drivers/infiniband/core/netlink.c | 2 +- include/uapi/rdma/rdma_netlink.h | 39 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 39 insertions(+), 2 deletions(-)
drivers/infiniband/core/netlink.c +1 -1
@@ -61,7 +61,7 @@ static bool is_nl_msg_valid(unsigned int type, unsigned int op)RDMA_NL_IWPM_NUM_OPS ,0 ,RDMA_NL_LS_NUM_OPS , - 0 }; + RDMA_NLDEV_NUM_OPS };/* * This BUILD_BUG_ON is intended to catch addition of new
include/uapi/rdma/rdma_netlink.h +38 -1
@@ -8,7 +8,7 @@ enum {RDMA_NL_IWCM ,RDMA_NL_RSVD ,RDMA_NL_LS ,/* RDMA Local Services */ -RDMA_NL_I40IW , +RDMA_NL_NLDEV ,/* RDMA device interface */ RDMA_NL_NUM_CLIENTS };
@@ -222,4 +222,41 @@ struct rdma_nla_ls_gid { __u8 gid[16]; }; +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 , + +RDMA_NLDEV_NUM_OPS +}; + +enum rdma_nldev_attr { +/* don't change the order or add anything between, this is ABI! */ +RDMA_NLDEV_ATTR_UNSPEC , + +/* Identifier for ib_device */ +RDMA_NLDEV_ATTR_DEV_INDEX ,/* u32 */ + +RDMA_NLDEV_ATTR_DEV_NAME ,/* string */ +/* +* Device index together with port index are identifiers +* for port/link properties. +* +* For RDMA_NLDEV_CMD_GET comamnd, port index will return number +* of available ports in ib_device, while for port specific operations, +* it will be real port index as it appears in sysfs. Port index follows +* sysfs notation and starts from 1 for the first port. +*/ +RDMA_NLDEV_ATTR_PORT_INDEX ,/* u32 */ + +RDMA_NLDEV_ATTR_MAX +};#endif /* _UAPI_RDMA_NETLINK_H */
Signature · 6 lines
2.13.3 -- 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
PATCH 20/33 RDMA/netlink: Add nldev initialization flows
In reply to a message not in the archiveFrom: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
Add nldev init and exit flows to the RDMA/core.
--- drivers/infiniband/core/Makefile | 3 ++- drivers/infiniband/core/device.c | 2 ++ drivers/infiniband/core/nldev.c | 45 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 49 insertions(+), 1 deletion(-) create mode 100644 drivers/infiniband/core/nldev.c
drivers/infiniband/core/Makefile +2 -1
@@ -11,7 +11,8 @@ ib_core-y := packer.o ud_header.o verbs.o cq.o rw.o sysfs.o \ device.o fmr_pool.o cache.o netlink.o \ roce_gid_mgmt.o mr_pool.o addr.o sa_query.o \ multicast.o mad.o smi.o agent.o mad_rmpp.o \ - security.o + security.o nldev.o + ib_core-$(CONFIG_INFINIBAND_USER_MEM) += umem.o ib_core-$(CONFIG_INFINIBAND_ON_DEMAND_PAGING) += umem_odp.o umem_rbtree.o ib_core-$(CONFIG_CGROUP_RDMA) += cgroup.o
drivers/infiniband/core/device.c +2 -0
@@ -1212,6 +1212,7 @@ static int __init ib_core_init(void) goto err_sa; } +nldev_init ();rdma_nl_register (RDMA_NL_LS , ibnl_ls_cb_table);ib_cache_setup ();
@@ -1237,6 +1238,7 @@ static int __init ib_core_init(void) static void __exit ib_core_cleanup(void) {ib_cache_cleanup (); +nldev_exit ();rdma_nl_unregister (RDMA_NL_LS );unregister_lsm_notifier (&ibdev_lsm_nb);ib_sa_cleanup ();
drivers/infiniband/core/nldev.c new file +45 -0
45 lines not on this page · raw message
Signature · 6 lines
2.13.3 -- 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
PATCH 21/33 RDMA/netlink: Implement nldev device dumpit calback
In reply to a message not in the archiveFrom: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
This patch adds the ability to return all available devices together with their properties.
--- drivers/infiniband/core/nldev.c | 64 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 63 insertions(+), 1 deletion(-)
drivers/infiniband/core/nldev.c +63 -1
@@ -30,13 +30,75 @@* POSSIBILITY OF SUCH DAMAGE. */ +#include <net/netlink.h> #include <rdma/rdma_netlink.h> #include "core_priv.h" +static const struct nla_policy nldev_policy[RDMA_NLDEV_ATTR_MAX ] = { + [RDMA_NLDEV_ATTR_DEV_INDEX ] = { .type =NLA_U32 }, + [RDMA_NLDEV_ATTR_DEV_NAME ] = { .type =NLA_NUL_STRING , + .len =IB_DEVICE_NAME_MAX -1 }, + [RDMA_NLDEV_ATTR_PORT_INDEX ] = { .type =NLA_U32 }, +}; + +static int fill_dev_info (struct sk_buff *msg,struct ib_device *device) +{ +if (nla_put_u32 (msg,RDMA_NLDEV_ATTR_DEV_INDEX , device->index)) +return -EMSGSIZE ; +if (nla_put_string (msg,RDMA_NLDEV_ATTR_DEV_NAME , device->name)) +return -EMSGSIZE ; +if (nla_put_u32 (msg,RDMA_NLDEV_ATTR_PORT_INDEX ,rdma_end_port (device))) +return -EMSGSIZE ; +return 0 ; +} + +static int _nldev_get_dumpit (struct ib_device *device, +struct sk_buff *skb, +struct netlink_callback *cb, +unsigned int idx) +{ +int start = cb->args[0 ]; +struct nlmsghdr *nlh; + +if (idx < start) +return 0 ; + + nlh =nlmsg_put (skb,NETLINK_CB (cb->skb).portid, cb->nlh->nlmsg_seq, +RDMA_NL_GET_TYPE (RDMA_NL_NLDEV ,RDMA_NLDEV_CMD_GET ), +0 ,NLM_F_MULTI ); + +if (fill_dev_info (skb, device)) { +nlmsg_cancel (skb, nlh); + goto out; + } + +nlmsg_end (skb, nlh); + + idx++; + +out: cb->args[0 ] = idx; +return skb->len; +} + +static int nldev_get_dumpit (struct sk_buff *skb,struct netlink_callback *cb) +{ +/* +* There is no need to take lock, because +* we are relying on ib_core's lists_rwsem +*/ +return ib_enum_all_devs (_nldev_get_dumpit, skb, cb); +} + +static const struct rdma_nl_cbs nldev_cb_table[] = { + [RDMA_NLDEV_CMD_GET ] = { + .dump = nldev_get_dumpit, + }, +}; +void __init nldev_init (void ) { -rdma_nl_register (RDMA_NL_NLDEV ,NULL ); +rdma_nl_register (RDMA_NL_NLDEV , nldev_cb_table); } void __exit nldev_exit(void)
Signature · 6 lines
2.13.3 -- 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
PATCH 22/33 RDMA/netlink: Add nldev device doit implementation
In reply to a message not in the archiveFrom: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
Provide ability to query specific device.
--- drivers/infiniband/core/nldev.c | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+)
drivers/infiniband/core/nldev.c +40 -0
@@ -53,6 +53,45 @@ static int fill_dev_info(struct sk_buff *msg, struct ib_device *device)return 0 ; } +static int nldev_get_doit (struct sk_buff *skb,struct nlmsghdr *nlh, +struct netlink_ext_ack *extack) +{ +struct nlattr *tb[RDMA_NLDEV_ATTR_MAX ]; +struct ib_device *device; +struct sk_buff *msg; +u32 index; +int err; + + err =nlmsg_parse (nlh,0 , tb,RDMA_NLDEV_ATTR_MAX -1 , + nldev_policy, extack); +if (err || !tb[RDMA_NLDEV_ATTR_DEV_INDEX ]) +return -EINVAL ; + + index =nla_get_u32 (tb[RDMA_NLDEV_ATTR_DEV_INDEX ]); + + device =__ib_device_get_by_index (index); +if (!device) +return -EINVAL ; + + msg =nlmsg_new (NLMSG_DEFAULT_SIZE ,GFP_KERNEL ); +if (!msg) +return -ENOMEM ; + + nlh =nlmsg_put (msg,NETLINK_CB (skb).portid, nlh->nlmsg_seq, +RDMA_NL_GET_TYPE (RDMA_NL_NLDEV ,RDMA_NLDEV_CMD_GET ), +0 ,0 ); + + err =fill_dev_info (msg, device); +if (err) { +nlmsg_free (msg); +return err; + } + +nlmsg_end (msg, nlh); + +return rdma_nl_unicast (msg,NETLINK_CB (skb).portid); +} + static int _nldev_get_dumpit(struct ib_device *device, struct sk_buff *skb, struct netlink_callback *cb,
@@ -92,6 +131,7 @@ static int nldev_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb)static const struct rdma_nl_cbs nldev_cb_table[] = { [RDMA_NLDEV_CMD_GET ] = { + .doit = nldev_get_doit, .dump = nldev_get_dumpit, }, };
Signature · 6 lines
2.13.3 -- 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
PATCH 23/33 RDMA/netlink: Add nldev port dumpit implementation
In reply to a message not in the archiveFrom: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
This patch implements the query interface to get all ports data for the specific device.
--- drivers/infiniband/core/nldev.c | 61 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+)
drivers/infiniband/core/nldev.c +61 -0
@@ -53,6 +53,18 @@ static int fill_dev_info(struct sk_buff *msg, struct ib_device *device)return 0 ; } +static int fill_port_info (struct sk_buff *msg, +struct ib_device *device,u32 port) +{ +if (nla_put_u32 (msg,RDMA_NLDEV_ATTR_DEV_INDEX , device->index)) +return -EMSGSIZE ; +if (nla_put_string (msg,RDMA_NLDEV_ATTR_DEV_NAME , device->name)) +return -EMSGSIZE ; +if (nla_put_u32 (msg,RDMA_NLDEV_ATTR_PORT_INDEX , port)) +return -EMSGSIZE ; +return 0 ; +} +static int nldev_get_doit (struct sk_buff *skb,struct nlmsghdr *nlh,struct netlink_ext_ack *extack) {
@@ -129,11 +141,60 @@ static int nldev_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb)return ib_enum_all_devs (_nldev_get_dumpit, skb, cb); } +static int nldev_port_get_dumpit (struct sk_buff *skb, +struct netlink_callback *cb) +{ +struct nlattr *tb[RDMA_NLDEV_ATTR_MAX ]; +struct ib_device *device; +int start = cb->args[0 ]; +struct nlmsghdr *nlh; +u32 idx =0 ; +u32 ifindex; +int err; +u32 p; + + err =nlmsg_parse (cb->nlh,0 , tb,RDMA_NLDEV_ATTR_MAX -1 , + nldev_policy,NULL ); +if (err || !tb[RDMA_NLDEV_ATTR_DEV_INDEX ]) +return -EINVAL ; + + ifindex =nla_get_u32 (tb[RDMA_NLDEV_ATTR_DEV_INDEX ]); + device =__ib_device_get_by_index (ifindex); +if (!device) +return -EINVAL ; + +for (p =rdma_start_port (device); p <=rdma_end_port (device); ++p) { +if (idx < start) { + idx++; +continue ; + } + + nlh =nlmsg_put (skb,NETLINK_CB (cb->skb).portid, + cb->nlh->nlmsg_seq, +RDMA_NL_GET_TYPE (RDMA_NL_NLDEV , +RDMA_NLDEV_CMD_PORT_GET ), +0 ,NLM_F_MULTI ); + +if (fill_port_info (skb, device, p)) { +nlmsg_cancel (skb, nlh); + goto out; + } + idx++; +nlmsg_end (skb, nlh); + } + +out: cb->args[0 ] = idx; +return skb->len; +} +static const struct rdma_nl_cbs nldev_cb_table[] = { [RDMA_NLDEV_CMD_GET ] = { .doit = nldev_get_doit, .dump = nldev_get_dumpit, }, + [RDMA_NLDEV_CMD_PORT_GET ] = { + .dump = nldev_port_get_dumpit, + }, };void __init nldev_init (void )
Signature · 6 lines
2.13.3 -- 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
PATCH 24/33 RDMA/netlink: Implement nldev port doit callback
In reply to a message not in the archiveFrom: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
Provide ability to get specific to device and port information.
--- drivers/infiniband/core/nldev.c | 44 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+)
drivers/infiniband/core/nldev.c +44 -0
@@ -141,6 +141,49 @@ static int nldev_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb)return ib_enum_all_devs (_nldev_get_dumpit, skb, cb); } +static int nldev_port_get_doit (struct sk_buff *skb,struct nlmsghdr *nlh, +struct netlink_ext_ack *extack) +{ +struct nlattr *tb[RDMA_NLDEV_ATTR_MAX ]; +struct ib_device *device; +struct sk_buff *msg; +u32 index; +u32 port; +int err; + + err =nlmsg_parse (nlh,0 , tb,RDMA_NLDEV_ATTR_MAX -1 , + nldev_policy, extack); +if (err || !tb[RDMA_NLDEV_ATTR_PORT_INDEX ]) +return -EINVAL ; + + index =nla_get_u32 (tb[RDMA_NLDEV_ATTR_DEV_INDEX ]); + device =__ib_device_get_by_index (index); +if (!device) +return -EINVAL ; + + port =nla_get_u32 (tb[RDMA_NLDEV_ATTR_PORT_INDEX ]); +if (!rdma_is_port_valid (device, port)) +return -EINVAL ; + + msg =nlmsg_new (NLMSG_DEFAULT_SIZE ,GFP_KERNEL ); +if (!msg) +return -ENOMEM ; + + nlh =nlmsg_put (msg,NETLINK_CB (skb).portid, nlh->nlmsg_seq, +RDMA_NL_GET_TYPE (RDMA_NL_NLDEV ,RDMA_NLDEV_CMD_GET ), +0 ,0 ); + + err =fill_port_info (msg, device, port); +if (err) { +nlmsg_free (msg); +return err; + } + +nlmsg_end (msg, nlh); + +return rdma_nl_unicast (msg,NETLINK_CB (skb).portid); +} +static int nldev_port_get_dumpit (struct sk_buff *skb,struct netlink_callback *cb) {
@@ -193,6 +236,7 @@ static const struct rdma_nl_cbs nldev_cb_table[] = { .dump = nldev_get_dumpit, }, [RDMA_NLDEV_CMD_PORT_GET] = { + .doit = nldev_port_get_doit, .dump = nldev_port_get_dumpit, }, };
Signature · 6 lines
2.13.3 -- 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
PATCH 25/33 RDMA/netlink: Expose device and port capability masks
In reply to a message not in the archiveFrom: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
The port capability mask is exposed to user space via sysfs interface, while device capabilities are available for verbs only.
This patch provides those capabilities through netlink interface.
--- drivers/infiniband/core/nldev.c | 19 +++++++++++++++++++ include/uapi/rdma/rdma_netlink.h | 5 +++++ 2 files changed, 24 insertions(+)
drivers/infiniband/core/nldev.c +19 -0
@@ -50,18 +50,37 @@ static int fill_dev_info(struct sk_buff *msg, struct ib_device *device)return -EMSGSIZE ;if (nla_put_u32 (msg,RDMA_NLDEV_ATTR_PORT_INDEX ,rdma_end_port (device)))return -EMSGSIZE ; + +BUILD_BUG_ON (sizeof (device->attrs.device_cap_flags) !=sizeof (u64)); +if (nla_put_u64_64bit (msg,RDMA_NLDEV_ATTR_CAP_FLAGS , + device->attrs.device_cap_flags,0 )) +return -EMSGSIZE ; +return 0 ; }static int fill_port_info (struct sk_buff *msg,struct ib_device *device,u32 port) { +struct ib_port_attr attr; +int ret; +if (nla_put_u32 (msg,RDMA_NLDEV_ATTR_DEV_INDEX , device->index))return -EMSGSIZE ;if (nla_put_string (msg,RDMA_NLDEV_ATTR_DEV_NAME , device->name))return -EMSGSIZE ;if (nla_put_u32 (msg,RDMA_NLDEV_ATTR_PORT_INDEX , port))return -EMSGSIZE ; + + ret =ib_query_port (device, port, &attr); +if (ret) +return ret; + +BUILD_BUG_ON (sizeof (attr.port_cap_flags) >sizeof (u64)); +if (nla_put_u64_64bit (msg,RDMA_NLDEV_ATTR_CAP_FLAGS , + (u64 )attr.port_cap_flags,0 )) +return -EMSGSIZE ; +return 0 ; }
include/uapi/rdma/rdma_netlink.h +5 -0
@@ -257,6 +257,11 @@ enum rdma_nldev_attr {*/ RDMA_NLDEV_ATTR_PORT_INDEX ,/* u32 */ +/* +* Device and port capabilities +*/ +RDMA_NLDEV_ATTR_CAP_FLAGS ,/* u64 */ +RDMA_NLDEV_ATTR_MAX };#endif /* _UAPI_RDMA_NETLINK_H */
Signature · 6 lines
2.13.3 -- 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
PATCH 26/33 RDMA: Simplify get firmware interface
In reply to a message not in the archiveFrom: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
There is a need to forward FW version to user space application through RDMA netlink. In order to make it safe, there is need to declare nla_policy and limit the size of FW string.
The new define IB_FW_VERSION_NAME_MAX will limit the size of FW version string. That define was chosen to be equal to ETHTOOL_FWVERS_LEN, because many drivers anyway are limited by that value indirectly.
The introduction of this define allows us to remove the string size from get_fw_str function signature.
--- drivers/infiniband/core/device.c | 4 ++-- drivers/infiniband/core/sysfs.c | 4 ++-- drivers/infiniband/hw/cxgb3/iwch_provider.c | 5 ++--- drivers/infiniband/hw/cxgb4/provider.c | 5 ++--- drivers/infiniband/hw/hfi1/verbs.c | 5 ++--- drivers/infiniband/hw/i40iw/i40iw_verbs.c | 7 +++---- drivers/infiniband/hw/mlx4/main.c | 5 ++--- drivers/infiniband/hw/mlx5/main.c | 8 ++++---- drivers/infiniband/hw/mthca/mthca_provider.c | 5 ++--- drivers/infiniband/hw/nes/nes_verbs.c | 5 ++--- drivers/infiniband/hw/ocrdma/ocrdma_main.c | 5 ++--- drivers/infiniband/hw/qedr/main.c | 5 ++--- drivers/infiniband/hw/usnic/usnic_ib_main.c | 6 ++---- drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c | 5 ++--- drivers/infiniband/ulp/ipoib/ipoib_ethtool.c | 3 +-- include/rdma/ib_verbs.h | 6 ++++-- 16 files changed, 36 insertions(+), 47 deletions(-)
drivers/infiniband/core/device.c +2 -2
@@ -336,10 +336,10 @@ static int read_port_immutable(struct ib_device *device)return 0 ; } -void ib_get_device_fw_str (struct ib_device *dev,char *str,size_t str_len) +void ib_get_device_fw_str (struct ib_device *dev,char *str) {if (dev->get_dev_fw_str) - dev->get_dev_fw_str (dev, str, str_len); + dev->get_dev_fw_str (dev, str);else str[0 ] ='\0' ; }
drivers/infiniband/core/sysfs.c +2 -2
@@ -1210,8 +1210,8 @@ static ssize_t show_fw_ver(struct device *device, struct device_attribute *attr, { struct ib_device *dev = container_of(device, struct ib_device, dev); -ib_get_device_fw_str (dev, buf,PAGE_SIZE ); -strlcat (buf,"\n" ,PAGE_SIZE ); +ib_get_device_fw_str (dev, buf); +strlcat (buf,"\n" ,IB_FW_VERSION_NAME_MAX );return strlen (buf); }
drivers/infiniband/hw/cxgb3/iwch_provider.c +2 -3
@@ -1336,8 +1336,7 @@ static int iwch_port_immutable(struct ib_device *ibdev, u8 port_num,return 0 ; } -static void get_dev_fw_ver_str (struct ib_device *ibdev,char *str, -size_t str_len) +static void get_dev_fw_ver_str (struct ib_device *ibdev,char *str) {struct iwch_dev *iwch_dev =to_iwch_dev (ibdev);struct ethtool_drvinfo info;
@@ -1345,7 +1344,7 @@ static void get_dev_fw_ver_str(struct ib_device *ibdev, char *str,pr_debug ("%s dev 0x%p\n" , __func__, iwch_dev); lldev->ethtool_ops->get_drvinfo (lldev, &info); -snprintf (str, str_len,"%s" , info.fw_version); +snprintf (str,IB_FW_VERSION_NAME_MAX ,"%s" , info.fw_version); }int iwch_register_device (struct iwch_dev *dev)
drivers/infiniband/hw/cxgb4/provider.c +2 -3
@@ -517,14 +517,13 @@ static int c4iw_port_immutable(struct ib_device *ibdev, u8 port_num,return 0 ; } -static void get_dev_fw_str (struct ib_device *dev,char *str, -size_t str_len) +static void get_dev_fw_str (struct ib_device *dev,char *str) { struct c4iw_dev *c4iw_dev = container_of(dev, struct c4iw_dev, ibdev);pr_debug ("%s dev 0x%p\n" , __func__, dev); - snprintf(str, str_len, "%u.%u.%u.%u", + snprintf(str, IB_FW_VERSION_NAME_MAX, "%u.%u.%u.%u", FW_HDR_FW_VER_MAJOR_G(c4iw_dev->rdev.lldi.fw_vers), FW_HDR_FW_VER_MINOR_G(c4iw_dev->rdev.lldi.fw_vers), FW_HDR_FW_VER_MICRO_G(c4iw_dev->rdev.lldi.fw_vers),
drivers/infiniband/hw/hfi1/verbs.c +2 -3
@@ -1561,14 +1561,13 @@ static void init_ibport(struct hfi1_pportdata *ppd)RCU_INIT_POINTER (ibp->rvp.qp[1 ],NULL ); } -static void hfi1_get_dev_fw_str (struct ib_device *ibdev,char *str, -size_t str_len) +static void hfi1_get_dev_fw_str (struct ib_device *ibdev,char *str) {struct rvt_dev_info *rdi =ib_to_rvt (ibdev);struct hfi1_ibdev *dev =dev_from_rdi (rdi);u32 ver =dd_from_dev (dev)->dc8051_ver; -snprintf (str, str_len,"%u.%u.%u" ,dc8051_ver_maj (ver), +snprintf (str,IB_FW_VERSION_NAME_MAX ,"%u.%u.%u" ,dc8051_ver_maj (ver),dc8051_ver_min (ver),dc8051_ver_patch (ver)); }
drivers/infiniband/hw/i40iw/i40iw_verbs.c +3 -4
@@ -2584,13 +2584,12 @@ static const char * const i40iw_hw_stat_names[] = { "iwRdmaInv" }; -static void i40iw_get_dev_fw_str (struct ib_device *dev,char *str, -size_t str_len) +static void i40iw_get_dev_fw_str (struct ib_device *dev,char *str) {u32 firmware_version =I40IW_FW_VERSION ; -snprintf (str, str_len,"%u.%u" , firmware_version, - (firmware_version &0x000000ff )); +snprintf (str,IB_FW_VERSION_NAME_MAX ,"%u.%u" , firmware_version, + (firmware_version &0x000000ff )); }/**
drivers/infiniband/hw/mlx4/main.c +2 -3
@@ -2587,12 +2587,11 @@ static int mlx4_port_immutable(struct ib_device *ibdev, u8 port_num,return 0 ; } -static void get_fw_ver_str (struct ib_device *device,char *str, -size_t str_len) +static void get_fw_ver_str (struct ib_device *device,char *str) {struct mlx4_ib_dev *dev = container_of(device, struct mlx4_ib_dev, ib_dev); -snprintf (str, str_len,"%d.%d.%d" , +snprintf (str,IB_FW_VERSION_NAME_MAX ,"%d.%d.%d" , (int ) (dev->dev->caps.fw_ver >>32 ), (int ) (dev->dev->caps.fw_ver >>16 ) &0xffff , (int ) dev->dev->caps.fw_ver &0xffff );
drivers/infiniband/hw/mlx5/main.c +4 -4
@@ -3285,13 +3285,13 @@ static int mlx5_port_immutable(struct ib_device *ibdev, u8 port_num,return 0 ; } -static void get_dev_fw_str (struct ib_device *ibdev,char *str, -size_t str_len) +static void get_dev_fw_str (struct ib_device *ibdev,char *str) {struct mlx5_ib_dev *dev = container_of(ibdev, struct mlx5_ib_dev, ib_dev); -snprintf (str, str_len,"%d.%d.%04d" ,fw_rev_maj (dev->mdev), -fw_rev_min (dev->mdev),fw_rev_sub (dev->mdev)); +snprintf (str,IB_FW_VERSION_NAME_MAX ,"%d.%d.%04d" , +fw_rev_maj (dev->mdev),fw_rev_min (dev->mdev), +fw_rev_sub (dev->mdev)); }static int mlx5_eth_lag_init (struct mlx5_ib_dev *dev)
drivers/infiniband/hw/mthca/mthca_provider.c +2 -3
@@ -1178,12 +1178,11 @@ static int mthca_port_immutable(struct ib_device *ibdev, u8 port_num,return 0 ; } -static void get_dev_fw_str (struct ib_device *device,char *str, -size_t str_len) +static void get_dev_fw_str (struct ib_device *device,char *str) {struct mthca_dev *dev = container_of(device, struct mthca_dev, ib_dev); -snprintf (str, str_len,"%d.%d.%d" , +snprintf (str,IB_FW_VERSION_NAME_MAX ,"%d.%d.%d" , (int ) (dev->fw_ver >>32 ), (int ) (dev->fw_ver >>16 ) &0xffff , (int ) dev->fw_ver &0xffff );
drivers/infiniband/hw/nes/nes_verbs.c +2 -3
@@ -3672,15 +3672,14 @@ static int nes_port_immutable(struct ib_device *ibdev, u8 port_num,return 0 ; } -static void get_dev_fw_str (struct ib_device *dev,char *str, -size_t str_len) +static void get_dev_fw_str (struct ib_device *dev,char *str) {struct nes_ib_device *nesibdev = container_of(dev, struct nes_ib_device, ibdev);struct nes_vnic *nesvnic = nesibdev->nesvnic;nes_debug (NES_DBG_INIT ,"\n" ); -snprintf (str, str_len,"%u.%u" , +snprintf (str,IB_FW_VERSION_NAME_MAX ,"%u.%u" , (nesvnic->nesdev->nesadapter->firmware_version >>16 ), (nesvnic->nesdev->nesadapter->firmware_version &0x000000ff )); }
drivers/infiniband/hw/ocrdma/ocrdma_main.c +2 -3
@@ -107,12 +107,11 @@ static int ocrdma_port_immutable(struct ib_device *ibdev, u8 port_num,return 0 ; } -static void get_dev_fw_str (struct ib_device *device,char *str, -size_t str_len) +static void get_dev_fw_str (struct ib_device *device,char *str) {struct ocrdma_dev *dev =get_ocrdma_dev (device); -snprintf (str, str_len,"%s" , &dev->attr.fw_ver[0 ]); +snprintf (str,IB_FW_VERSION_NAME_MAX ,"%s" , &dev->attr.fw_ver[0 ]); }static int ocrdma_register_device (struct ocrdma_dev *dev)
drivers/infiniband/hw/qedr/main.c +2 -3
@@ -68,13 +68,12 @@ static enum rdma_link_layer qedr_link_layer(struct ib_device *device,return IB_LINK_LAYER_ETHERNET ; } -static void qedr_get_dev_fw_str (struct ib_device *ibdev,char *str, -size_t str_len) +static void qedr_get_dev_fw_str (struct ib_device *ibdev,char *str) {struct qedr_dev *qedr =get_qedr_dev (ibdev);u32 fw_ver = (u32 )qedr->attr.fw_ver; -snprintf (str, str_len,"%d. %d. %d. %d" , +snprintf (str,IB_FW_VERSION_NAME_MAX ,"%d. %d. %d. %d" , (fw_ver >>24 ) &0xFF , (fw_ver >>16 ) &0xFF , (fw_ver >>8 ) &0xFF , fw_ver &0xFF ); }
drivers/infiniband/hw/usnic/usnic_ib_main.c +2 -4
@@ -333,9 +333,7 @@ static int usnic_port_immutable(struct ib_device *ibdev, u8 port_num,return 0 ; } -static void usnic_get_dev_fw_str (struct ib_device *device, -char *str, -size_t str_len) +static void usnic_get_dev_fw_str (struct ib_device *device,char *str) {struct usnic_ib_dev *us_ibdev = container_of(device, struct usnic_ib_dev, ib_dev);
@@ -345,7 +343,7 @@ static void usnic_get_dev_fw_str(struct ib_device *device, us_ibdev->netdev->ethtool_ops->get_drvinfo (us_ibdev->netdev, &info);mutex_unlock (&us_ibdev->usdev_lock); -snprintf (str, str_len,"%s" , info.fw_version); +snprintf (str,IB_FW_VERSION_NAME_MAX ,"%s" , info.fw_version); }/* Start of PF discovery section */
drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c +2 -3
@@ -102,12 +102,11 @@ static struct device_attribute *pvrdma_class_attributes[] = { &dev_attr_board_id }; -static void pvrdma_get_fw_ver_str (struct ib_device *device,char *str, -size_t str_len) +static void pvrdma_get_fw_ver_str (struct ib_device *device,char *str) {struct pvrdma_dev *dev = container_of(device, struct pvrdma_dev, ib_dev); -snprintf (str, str_len,"%d.%d.%d\n" , +snprintf (str,IB_FW_VERSION_NAME_MAX ,"%d.%d.%d\n" , (int ) (dev->dsr->caps.fw_ver >>32 ), (int ) (dev->dsr->caps.fw_ver >>16 ) &0xffff , (int ) dev->dsr->caps.fw_ver &0xffff );
drivers/infiniband/ulp/ipoib/ipoib_ethtool.c +1 -2
@@ -62,8 +62,7 @@ static void ipoib_get_drvinfo(struct net_device *netdev, {struct ipoib_dev_priv *priv =ipoib_priv (netdev); -ib_get_device_fw_str (priv->ca, drvinfo->fw_version, -sizeof (drvinfo->fw_version)); +ib_get_device_fw_str (priv->ca, drvinfo->fw_version);strlcpy (drvinfo->bus_info,dev_name (priv->ca->dev.parent),sizeof (drvinfo->bus_info));
include/rdma/ib_verbs.h +4 -2
@@ -64,6 +64,8 @@#include <linux/cgroup_rdma.h> #include <uapi/rdma/ib_user_verbs.h> +#define IB_FW_VERSION_NAME_MAX ETHTOOL_FWVERS_LEN +extern struct workqueue_struct *ib_wq;extern struct workqueue_struct *ib_comp_wq;
@@ -2307,7 +2309,7 @@ struct ib_device {* in fast paths. */ int (*get_port_immutable)(struct ib_device *,u8 ,struct ib_port_immutable *); -void (*get_dev_fw_str)(struct ib_device *,char *str,size_t str_len); +void (*get_dev_fw_str)(struct ib_device *,char *str); };struct ib_client {
@@ -2343,7 +2345,7 @@ struct ib_client {struct ib_device *ib_alloc_device (size_t size);void ib_dealloc_device (struct ib_device *device); -void ib_get_device_fw_str (struct ib_device *device,char *str,size_t str_len); +void ib_get_device_fw_str (struct ib_device *device,char *str); int ib_register_device(struct ib_device *device, int (*port_callback)(struct ib_device *,
Signature · 6 lines
2.13.3 -- 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
PATCH 27/33 RDMA/netlink: Export FW version
In reply to a message not in the archiveFrom: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
Add FW version to the device properties exported
by RDMA netlink, to be used by RDMAtool.
--- drivers/infiniband/core/nldev.c | 9 +++++++++ include/uapi/rdma/rdma_netlink.h | 4 ++++ 2 files changed, 13 insertions(+)
drivers/infiniband/core/nldev.c +9 -0
@@ -40,10 +40,14 @@ static const struct nla_policy nldev_policy[RDMA_NLDEV_ATTR_MAX] = { [RDMA_NLDEV_ATTR_DEV_NAME ] = { .type =NLA_NUL_STRING , .len = IB_DEVICE_NAME_MAX - 1}, [RDMA_NLDEV_ATTR_PORT_INDEX] = { .type = NLA_U32 }, + [RDMA_NLDEV_ATTR_FW_VERSION ] = { .type =NLA_NUL_STRING , + .len = IB_FW_VERSION_NAME_MAX - 1}, };static int fill_dev_info (struct sk_buff *msg,struct ib_device *device) { +char fw[IB_FW_VERSION_NAME_MAX ]; +if (nla_put_u32 (msg,RDMA_NLDEV_ATTR_DEV_INDEX , device->index))return -EMSGSIZE ;if (nla_put_string (msg,RDMA_NLDEV_ATTR_DEV_NAME , device->name))
@@ -56,6 +60,11 @@ static int fill_dev_info(struct sk_buff *msg, struct ib_device *device) device->attrs.device_cap_flags, 0))return -EMSGSIZE ; +ib_get_device_fw_str (device, fw); +/* Device without FW has strlen(fw) */ +if (strlen (fw) &&nla_put_string (msg,RDMA_NLDEV_ATTR_FW_VERSION , fw)) +return -EMSGSIZE ; +return 0 ; }
include/uapi/rdma/rdma_netlink.h +4 -0
@@ -262,6 +262,10 @@ enum rdma_nldev_attr {*/ RDMA_NLDEV_ATTR_CAP_FLAGS ,/* u64 */ +/* +* FW version +*/ +RDMA_NLDEV_ATTR_FW_VERSION ,/* string */ RDMA_NLDEV_ATTR_MAX };#endif /* _UAPI_RDMA_NETLINK_H */
Signature · 6 lines
2.13.3 -- 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
PATCH 28/33 RDMA/netlink: Export node_guid and sys_image_guid
In reply to a message not in the archiveFrom: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
Add Node GUID and system image GUID to the device properties exported by RDMA netlink, to be used by RDMAtool.
--- drivers/infiniband/core/nldev.c | 8 ++++++++ include/uapi/rdma/rdma_netlink.h | 13 +++++++++++++ 2 files changed, 21 insertions(+)
drivers/infiniband/core/nldev.c +8 -0
@@ -42,6 +42,8 @@ static const struct nla_policy nldev_policy[RDMA_NLDEV_ATTR_MAX] = { [RDMA_NLDEV_ATTR_PORT_INDEX] = { .type = NLA_U32 }, [RDMA_NLDEV_ATTR_FW_VERSION ] = { .type =NLA_NUL_STRING , .len = IB_FW_VERSION_NAME_MAX - 1}, + [RDMA_NLDEV_ATTR_NODE_GUID] = { .type = NLA_U64 }, + [RDMA_NLDEV_ATTR_SYS_IMAGE_GUID] = { .type = NLA_U64 }, };static int fill_dev_info (struct sk_buff *msg,struct ib_device *device)
@@ -65,6 +67,12 @@ static int fill_dev_info(struct sk_buff *msg, struct ib_device *device)if (strlen (fw) &&nla_put_string (msg,RDMA_NLDEV_ATTR_FW_VERSION , fw))return -EMSGSIZE ; +if (nla_put_u64_64bit (msg,RDMA_NLDEV_ATTR_NODE_GUID , +be64_to_cpu (device->node_guid),0 )) +return -EMSGSIZE ; +if (nla_put_u64_64bit (msg,RDMA_NLDEV_ATTR_SYS_IMAGE_GUID , +be64_to_cpu (device->attrs.sys_image_guid),0 )) +return -EMSGSIZE ;return 0 ; }
include/uapi/rdma/rdma_netlink.h +13 -0
@@ -266,6 +266,19 @@ enum rdma_nldev_attr {* FW version */ RDMA_NLDEV_ATTR_FW_VERSION ,/* string */ + +/* +* Node GUID (in host byte order) associated with the RDMA device. +*/ +RDMA_NLDEV_ATTR_NODE_GUID ,/* u64 */ + +/* +* System image GUID (in host byte order) associated with +* this RDMA device and other devices which are part of a +* single system. +*/ +RDMA_NLDEV_ATTR_SYS_IMAGE_GUID ,/* u64 */ +RDMA_NLDEV_ATTR_MAX };#endif /* _UAPI_RDMA_NETLINK_H */
Signature · 6 lines
2.13.3 -- 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
PATCH 29/33 RDMA/netlink: Advertise IB subnet prefix
In reply to a message not in the archiveFrom: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
Add IB subnet prefix to the port properties exported by RDMA netlink.
--- drivers/infiniband/core/nldev.c | 5 +++++ include/uapi/rdma/rdma_netlink.h | 5 +++++ 2 files changed, 10 insertions(+)
drivers/infiniband/core/nldev.c +5 -0
@@ -44,6 +44,7 @@ static const struct nla_policy nldev_policy[RDMA_NLDEV_ATTR_MAX] = { .len = IB_FW_VERSION_NAME_MAX - 1}, [RDMA_NLDEV_ATTR_NODE_GUID] = { .type = NLA_U64 }, [RDMA_NLDEV_ATTR_SYS_IMAGE_GUID] = { .type = NLA_U64 }, + [RDMA_NLDEV_ATTR_SUBNET_PREFIX] = { .type = NLA_U64 }, };static int fill_dev_info (struct sk_buff *msg,struct ib_device *device)
@@ -97,6 +98,10 @@ static int fill_port_info(struct sk_buff *msg,if (nla_put_u64_64bit (msg,RDMA_NLDEV_ATTR_CAP_FLAGS , (u64 )attr.port_cap_flags,0 ))return -EMSGSIZE ; +if (rdma_protocol_ib (device, port) && +nla_put_u64_64bit (msg,RDMA_NLDEV_ATTR_SUBNET_PREFIX , + attr.subnet_prefix,0 )) +return -EMSGSIZE ;return 0 ; }
include/uapi/rdma/rdma_netlink.h +5 -0
@@ -279,6 +279,11 @@ enum rdma_nldev_attr {*/ RDMA_NLDEV_ATTR_SYS_IMAGE_GUID ,/* u64 */ +/* +* Subnet prefix (in host byte order) +*/ +RDMA_NLDEV_ATTR_SUBNET_PREFIX ,/* u64 */ +RDMA_NLDEV_ATTR_MAX };#endif /* _UAPI_RDMA_NETLINK_H */
Signature · 6 lines
2.13.3 -- 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
PATCH 30/33 RDMA/netink: Export lids and sm_lids
In reply to a message not in the archiveFrom: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
According to the IB specification, the LID and SM_LID are 16-bit wide, but to support OmniPath users, export it as 32-bit value from the beginning.
--- drivers/infiniband/core/nldev.c | 9 ++++++++- include/uapi/rdma/rdma_netlink.h | 8 ++++++++ 2 files changed, 16 insertions(+), 1 deletion(-)
drivers/infiniband/core/nldev.c +8 -1
@@ -45,6 +45,8 @@ static const struct nla_policy nldev_policy[RDMA_NLDEV_ATTR_MAX] = { [RDMA_NLDEV_ATTR_NODE_GUID] = { .type = NLA_U64 }, [RDMA_NLDEV_ATTR_SYS_IMAGE_GUID] = { .type = NLA_U64 }, [RDMA_NLDEV_ATTR_SUBNET_PREFIX] = { .type = NLA_U64 }, + [RDMA_NLDEV_ATTR_LID] = { .type = NLA_U32 }, + [RDMA_NLDEV_ATTR_SM_LID] = { .type = NLA_U32 }, };static int fill_dev_info (struct sk_buff *msg,struct ib_device *device)
@@ -102,7 +104,12 @@ static int fill_port_info(struct sk_buff *msg,nla_put_u64_64bit (msg,RDMA_NLDEV_ATTR_SUBNET_PREFIX , attr.subnet_prefix, 0)) return -EMSGSIZE; - +if (rdma_protocol_ib (device, port)) { +if (nla_put_u32 (msg,RDMA_NLDEV_ATTR_LID , attr.lid)) +return -EMSGSIZE ; +if (nla_put_u32 (msg,RDMA_NLDEV_ATTR_SM_LID , attr.sm_lid)) +return -EMSGSIZE ; + }return 0 ; }
include/uapi/rdma/rdma_netlink.h +8 -0
@@ -284,6 +284,14 @@ enum rdma_nldev_attr {*/ RDMA_NLDEV_ATTR_SUBNET_PREFIX ,/* u64 */ +/* +* Local Identifier (LID), +* According to IB specification, It is 16-bit address assigned +* by the Subnet Manager. Extended to be 32-bit for OmniPath users. +*/ +RDMA_NLDEV_ATTR_LID ,/* u32 */ +RDMA_NLDEV_ATTR_SM_LID ,/* u32 */ +RDMA_NLDEV_ATTR_MAX };#endif /* _UAPI_RDMA_NETLINK_H */
Signature · 6 lines
2.13.3 -- 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
PATCH 31/33 RDMA/netlink: Export LID mask control (LMC)
In reply to a message not in the archiveFrom: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
--- drivers/infiniband/core/nldev.c | 3 +++ include/uapi/rdma/rdma_netlink.h | 5 +++++ 2 files changed, 8 insertions(+)
drivers/infiniband/core/nldev.c +3 -0
@@ -47,6 +47,7 @@ static const struct nla_policy nldev_policy[RDMA_NLDEV_ATTR_MAX] = { [RDMA_NLDEV_ATTR_SUBNET_PREFIX] = { .type = NLA_U64 }, [RDMA_NLDEV_ATTR_LID] = { .type = NLA_U32 }, [RDMA_NLDEV_ATTR_SM_LID] = { .type = NLA_U32 }, + [RDMA_NLDEV_ATTR_LMC] = { .type = NLA_U8 }, };static int fill_dev_info (struct sk_buff *msg,struct ib_device *device)
@@ -109,6 +110,8 @@ static int fill_port_info(struct sk_buff *msg,return -EMSGSIZE ;if (nla_put_u32 (msg,RDMA_NLDEV_ATTR_SM_LID , attr.sm_lid))return -EMSGSIZE ; +if (nla_put_u8 (msg,RDMA_NLDEV_ATTR_LMC , attr.lmc)) +return -EMSGSIZE ; }return 0 ; }
include/uapi/rdma/rdma_netlink.h +5 -0
@@ -292,6 +292,11 @@ enum rdma_nldev_attr {RDMA_NLDEV_ATTR_LID ,/* u32 */ RDMA_NLDEV_ATTR_SM_LID ,/* u32 */ +/* +* LID mask control (LMC) +*/ +RDMA_NLDEV_ATTR_LMC ,/* u8 */ +RDMA_NLDEV_ATTR_MAX };#endif /* _UAPI_RDMA_NETLINK_H */
Signature · 6 lines
2.13.3 -- 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
PATCH 32/33 RDMA/netlink: Provide port state and physical link state
In reply to a message not in the archiveFrom: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
Add port state and physical link state to the users of RDMA netlink.
--- drivers/infiniband/core/nldev.c | 6 ++++++ include/uapi/rdma/rdma_netlink.h | 3 +++ 2 files changed, 9 insertions(+)
drivers/infiniband/core/nldev.c +6 -0
@@ -48,6 +48,8 @@ static const struct nla_policy nldev_policy[RDMA_NLDEV_ATTR_MAX] = { [RDMA_NLDEV_ATTR_LID] = { .type = NLA_U32 }, [RDMA_NLDEV_ATTR_SM_LID] = { .type = NLA_U32 }, [RDMA_NLDEV_ATTR_LMC] = { .type = NLA_U8 }, + [RDMA_NLDEV_ATTR_PORT_STATE] = { .type = NLA_U8 }, + [RDMA_NLDEV_ATTR_PORT_PHYS_STATE] = { .type = NLA_U8 }, };static int fill_dev_info (struct sk_buff *msg,struct ib_device *device)
@@ -113,6 +115,10 @@ static int fill_port_info(struct sk_buff *msg,if (nla_put_u8 (msg,RDMA_NLDEV_ATTR_LMC , attr.lmc))return -EMSGSIZE ; } +if (nla_put_u8 (msg,RDMA_NLDEV_ATTR_PORT_STATE , attr.state)) +return -EMSGSIZE ; +if (nla_put_u8 (msg,RDMA_NLDEV_ATTR_PORT_PHYS_STATE , attr.phys_state)) +return -EMSGSIZE ;return 0 ; }
include/uapi/rdma/rdma_netlink.h +3 -0
@@ -297,6 +297,9 @@ enum rdma_nldev_attr {*/ RDMA_NLDEV_ATTR_LMC ,/* u8 */ +RDMA_NLDEV_ATTR_PORT_STATE ,/* u8 */ +RDMA_NLDEV_ATTR_PORT_PHYS_STATE ,/* u8 */ +RDMA_NLDEV_ATTR_MAX };#endif /* _UAPI_RDMA_NETLINK_H */
Signature · 6 lines
2.13.3 -- 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
PATCH 33/33 RDMA/netlink: Export node_type
In reply to a message not in the archiveFrom: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
Add ability to get node_type for RDAM netlink users.
--- drivers/infiniband/core/nldev.c | 3 +++ include/uapi/rdma/rdma_netlink.h | 2 ++ 2 files changed, 5 insertions(+)
drivers/infiniband/core/nldev.c +3 -0
@@ -50,6 +50,7 @@ static const struct nla_policy nldev_policy[RDMA_NLDEV_ATTR_MAX] = { [RDMA_NLDEV_ATTR_LMC] = { .type = NLA_U8 }, [RDMA_NLDEV_ATTR_PORT_STATE] = { .type = NLA_U8 }, [RDMA_NLDEV_ATTR_PORT_PHYS_STATE] = { .type = NLA_U8 }, + [RDMA_NLDEV_ATTR_DEV_NODE_TYPE] = { .type = NLA_U8 }, };static int fill_dev_info (struct sk_buff *msg,struct ib_device *device)
@@ -79,6 +80,8 @@ static int fill_dev_info(struct sk_buff *msg, struct ib_device *device)if (nla_put_u64_64bit (msg,RDMA_NLDEV_ATTR_SYS_IMAGE_GUID ,be64_to_cpu (device->attrs.sys_image_guid),0 ))return -EMSGSIZE ; +if (nla_put_u8 (msg,RDMA_NLDEV_ATTR_DEV_NODE_TYPE , device->node_type)) +return -EMSGSIZE ;return 0 ; }
include/uapi/rdma/rdma_netlink.h +2 -0
@@ -300,6 +300,8 @@ enum rdma_nldev_attr {RDMA_NLDEV_ATTR_PORT_STATE ,/* u8 */ RDMA_NLDEV_ATTR_PORT_PHYS_STATE ,/* u8 */ +RDMA_NLDEV_ATTR_DEV_NODE_TYPE ,/* u8 */ +RDMA_NLDEV_ATTR_MAX };#endif /* _UAPI_RDMA_NETLINK_H */
Signature · 6 lines
2.13.3 -- 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