* net/smc and the RDMA core @ 2017-05-01 16:33 Christoph Hellwig 2017-05-01 17:29 ` Bart Van Assche ` (2 more replies) 0 siblings, 3 replies; 22+ messages in thread From: Christoph Hellwig @ 2017-05-01 16:33 UTC (permalink / raw) To: Ursula Braun, David S. Miller Cc: netdev-u79uwXL29TY76Z2rM5mHXA, linux-rdma-u79uwXL29TY76Z2rM5mHXA Hi Ursual, hi netdev reviewers, how did the smc protocol manage to get merged without any review on linux-rdma at all? As the results it seems it's very substandard in terms of RDMA API usage, e.g. it neither uses the proper CQ API nor the RDMA R/W API, and other will probably find additional issues as well. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: net/smc and the RDMA core 2017-05-01 16:33 net/smc and the RDMA core Christoph Hellwig @ 2017-05-01 17:29 ` Bart Van Assche [not found] ` <1493659776.2665.7.camel-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org> 2017-05-01 21:04 ` Steve Wise [not found] ` <20170501163311.GA22209-jcswGhMUV9g@public.gmane.org> 2 siblings, 1 reply; 22+ messages in thread From: Bart Van Assche @ 2017-05-01 17:29 UTC (permalink / raw) To: hch, davem, ubraun; +Cc: netdev, linux-rdma On Mon, 2017-05-01 at 18:33 +0200, Christoph Hellwig wrote: > Hi Ursual, hi netdev reviewers, > > how did the smc protocol manage to get merged without any review > on linux-rdma at all? As the results it seems it's very substandard > in terms of RDMA API usage, e.g. it neither uses the proper CQ API > nor the RDMA R/W API, and other will probably find additional issues > as well. Hello Dave and Ursula, It seems very rude to me to have merged the SMC protocol driver without having involved the linux-rdma community. Anyway, I have the following questions for Dave and Ursula: * Since the Linux kernel is standards based: where can we find the standard that defines the SMC wire protocol? If this protocol has not been standardized yet: in what file (other than *.[ch]) in the Linux kernel tree has this protocol been documented? * What are the differences between the SMC protocol, the SDP protocol and the rsockets protocol? How do existing implementations for these protocols compare to each other from a performance point of view? If no performance comparison between these protocols is available, shouldn't the performance of these protocols have been compared with each other before a review of the SMC driver even started? * What are the reasons why the SDP driver was never accepted upstream? Do the arguments why SDP was not accepted upstream also apply to the SMC driver (SDP = Sockets Direct Protocol)? * Since SMC has to be selected by specifying AF_SMC, how are users expected to specify whether AF_INET, AF_INET6 or yet another address family should be used to set up a connection between SMC endpoints? * Is the SMC driver limited to RoCE? Are you aware that the rsockets library supports multiple transport layers (RoCE, IB and iWARP)? * Since functionality that is similar what the SMC driver provides already exists in user space (rsockets), why has this functionality been reimplemented as a kernel driver (SMC)? Bart. ^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <1493659776.2665.7.camel-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>]
* RE: net/smc and the RDMA core [not found] ` <1493659776.2665.7.camel-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org> @ 2017-05-01 17:55 ` Parav Pandit [not found] ` <HE1PR0502MB30048AFD086C4B0D535BFC52D1140-692Kmc8YnlL9PhveBwpv4cDSnupUy6xnnBOFsp37pqbUKgpGm//BTAC/G2K4zDHf@public.gmane.org> 2017-05-02 12:34 ` Ursula Braun 1 sibling, 1 reply; 22+ messages in thread From: Parav Pandit @ 2017-05-01 17:55 UTC (permalink / raw) To: Bart Van Assche, hch-jcswGhMUV9g, davem-fT/PcQaiUtIeIZ0/mPfg9Q, ubraun-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8 Cc: netdev-u79uwXL29TY76Z2rM5mHXA, linux-rdma-u79uwXL29TY76Z2rM5mHXA Hi Bart, Ursula, Dave, I am particularly concerned about SMC as address family. It should not be treated as address family, but rather an additional protocol similar for socket type SOCK_STREAM. While doing performance benchmarking last month and while porting few database application, I encountered a major hurdle where user space library heavily depend on AF_INET and AF_INET6 family through get_addrinfo and other friend functions. Adding or treating AF_SMC as AF_INET just doesn't sound right. Most user space code doesn't care for the protocol field, but do handle domain field. I personally believe it's not too late to modify SMC to drop expose AF_SMC and have it exposed through new protocol that can be exposed through socket() API. Parav > -----Original Message----- > From: linux-rdma-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org [mailto:linux-rdma- > owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org] On Behalf Of Bart Van Assche > Sent: Monday, May 1, 2017 12:30 PM > To: hch-jcswGhMUV9g@public.gmane.org; davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org; ubraun-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org > Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org > Subject: Re: net/smc and the RDMA core > > On Mon, 2017-05-01 at 18:33 +0200, Christoph Hellwig wrote: > > Hi Ursual, hi netdev reviewers, > > > > how did the smc protocol manage to get merged without any review on > > linux-rdma at all? As the results it seems it's very substandard in > > terms of RDMA API usage, e.g. it neither uses the proper CQ API nor > > the RDMA R/W API, and other will probably find additional issues as > > well. > > Hello Dave and Ursula, > > It seems very rude to me to have merged the SMC protocol driver without > having involved the linux-rdma community. Anyway, I have the following > questions for Dave and Ursula: > * Since the Linux kernel is standards based: where can we find the standard > that defines the SMC wire protocol? If this protocol has not been > standardized yet: in what file (other than *.[ch]) in the Linux kernel > tree has this protocol been documented? > * What are the differences between the SMC protocol, the SDP protocol and > the rsockets protocol? How do existing implementations for these protocols > compare to each other from a performance point of view? If no performance > comparison between these protocols is available, shouldn't the performance > of these protocols have been compared with each other before a review of > the SMC driver even started? > * What are the reasons why the SDP driver was never accepted upstream? Do > the arguments why SDP was not accepted upstream also apply to the SMC > driver (SDP = Sockets Direct Protocol)? > * Since SMC has to be selected by specifying AF_SMC, how are users expected > to specify whether AF_INET, AF_INET6 or yet another address family should > be used to set up a connection between SMC endpoints? > * Is the SMC driver limited to RoCE? Are you aware that the rsockets library > supports multiple transport layers (RoCE, IB and iWARP)? > * Since functionality that is similar what the SMC driver provides already > exists in user space (rsockets), why has this functionality been > reimplemented as a kernel driver (SMC)? > > Bart.-- > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body > of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at > http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <HE1PR0502MB30048AFD086C4B0D535BFC52D1140-692Kmc8YnlL9PhveBwpv4cDSnupUy6xnnBOFsp37pqbUKgpGm//BTAC/G2K4zDHf@public.gmane.org>]
* Re: net/smc and the RDMA core [not found] ` <HE1PR0502MB30048AFD086C4B0D535BFC52D1140-692Kmc8YnlL9PhveBwpv4cDSnupUy6xnnBOFsp37pqbUKgpGm//BTAC/G2K4zDHf@public.gmane.org> @ 2017-05-02 12:41 ` Ursula Braun 2017-05-02 15:37 ` Bart Van Assche 0 siblings, 1 reply; 22+ messages in thread From: Ursula Braun @ 2017-05-02 12:41 UTC (permalink / raw) To: Parav Pandit, Bart Van Assche, hch-jcswGhMUV9g, davem-fT/PcQaiUtIeIZ0/mPfg9Q Cc: netdev-u79uwXL29TY76Z2rM5mHXA, linux-rdma-u79uwXL29TY76Z2rM5mHXA On 05/01/2017 07:55 PM, Parav Pandit wrote: > Hi Bart, Ursula, Dave, > > I am particularly concerned about SMC as address family. > It should not be treated as address family, but rather an additional protocol similar for socket type SOCK_STREAM. We tried to avoid changes of the kernel TCP code. A new address family seemed to be a feasible way to achieve this. > While doing performance benchmarking last month and while porting few database application, > > I encountered a major hurdle where user space library heavily depend on AF_INET and AF_INET6 family through get_addrinfo and other friend functions. > Adding or treating AF_SMC as AF_INET just doesn't sound right. > > Most user space code doesn't care for the protocol field, but do handle domain field. > > I personally believe it's not too late to modify SMC to drop expose AF_SMC and have it exposed through new protocol that can be exposed through socket() API. > > Parav > >> -----Original Message----- >> From: linux-rdma-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org [mailto:linux-rdma- >> owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org] On Behalf Of Bart Van Assche >> Sent: Monday, May 1, 2017 12:30 PM >> To: hch-jcswGhMUV9g@public.gmane.org; davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org; ubraun-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org >> Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org >> Subject: Re: net/smc and the RDMA core >> >> On Mon, 2017-05-01 at 18:33 +0200, Christoph Hellwig wrote: >>> Hi Ursual, hi netdev reviewers, >>> >>> how did the smc protocol manage to get merged without any review on >>> linux-rdma at all? As the results it seems it's very substandard in >>> terms of RDMA API usage, e.g. it neither uses the proper CQ API nor >>> the RDMA R/W API, and other will probably find additional issues as >>> well. >> >> Hello Dave and Ursula, >> >> It seems very rude to me to have merged the SMC protocol driver without >> having involved the linux-rdma community. Anyway, I have the following >> questions for Dave and Ursula: >> * Since the Linux kernel is standards based: where can we find the standard >> that defines the SMC wire protocol? If this protocol has not been >> standardized yet: in what file (other than *.[ch]) in the Linux kernel >> tree has this protocol been documented? >> * What are the differences between the SMC protocol, the SDP protocol and >> the rsockets protocol? How do existing implementations for these protocols >> compare to each other from a performance point of view? If no performance >> comparison between these protocols is available, shouldn't the performance >> of these protocols have been compared with each other before a review of >> the SMC driver even started? >> * What are the reasons why the SDP driver was never accepted upstream? Do >> the arguments why SDP was not accepted upstream also apply to the SMC >> driver (SDP = Sockets Direct Protocol)? >> * Since SMC has to be selected by specifying AF_SMC, how are users expected >> to specify whether AF_INET, AF_INET6 or yet another address family should >> be used to set up a connection between SMC endpoints? >> * Is the SMC driver limited to RoCE? Are you aware that the rsockets library >> supports multiple transport layers (RoCE, IB and iWARP)? >> * Since functionality that is similar what the SMC driver provides already >> exists in user space (rsockets), why has this functionality been >> reimplemented as a kernel driver (SMC)? >> >> Bart.-- >> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body >> of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at >> http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: net/smc and the RDMA core 2017-05-02 12:41 ` Ursula Braun @ 2017-05-02 15:37 ` Bart Van Assche 0 siblings, 0 replies; 22+ messages in thread From: Bart Van Assche @ 2017-05-02 15:37 UTC (permalink / raw) To: hch, davem, parav, ubraun; +Cc: netdev, linux-rdma On Tue, 2017-05-02 at 14:41 +0200, Ursula Braun wrote: > On 05/01/2017 07:55 PM, Parav Pandit wrote: > > Hi Bart, Ursula, Dave, > > > > I am particularly concerned about SMC as address family. > > It should not be treated as address family, but rather an additional > > protocol similar for socket type SOCK_STREAM. > > We tried to avoid changes of the kernel TCP code. A new address family > seemed to be a feasible way to achieve this. Hello Ursula, I agree with Parav that introducing a new address family for SMC was an unfortunate choice. As one can see in e.g. the implementation of the SCTP protocol no changes to the TCP implementation are needed to add support for a new SOCK_STREAM protocol. I think the SCTP implementation uses inet_register_protosw() to register itself dynamically. Bart. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: net/smc and the RDMA core [not found] ` <1493659776.2665.7.camel-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org> 2017-05-01 17:55 ` Parav Pandit @ 2017-05-02 12:34 ` Ursula Braun [not found] ` <dce14470-06f4-8da3-6894-cd724eac3447-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org> 1 sibling, 1 reply; 22+ messages in thread From: Ursula Braun @ 2017-05-02 12:34 UTC (permalink / raw) To: Bart Van Assche, hch-jcswGhMUV9g, davem-fT/PcQaiUtIeIZ0/mPfg9Q Cc: netdev-u79uwXL29TY76Z2rM5mHXA, linux-rdma-u79uwXL29TY76Z2rM5mHXA On 05/01/2017 07:29 PM, Bart Van Assche wrote: > On Mon, 2017-05-01 at 18:33 +0200, Christoph Hellwig wrote: >> Hi Ursual, hi netdev reviewers, >> >> how did the smc protocol manage to get merged without any review >> on linux-rdma at all? As the results it seems it's very substandard >> in terms of RDMA API usage, e.g. it neither uses the proper CQ API >> nor the RDMA R/W API, and other will probably find additional issues >> as well. > > Hello Dave and Ursula, > > It seems very rude to me to have merged the SMC protocol driver without > having involved the linux-rdma community. Anyway, I have the following > questions for Dave and Ursula: > * Since the Linux kernel is standards based: where can we find the standard > that defines the SMC wire protocol? If this protocol has not been > standardized yet: in what file (other than *.[ch]) in the Linux kernel > tree has this protocol been documented? Hello Bart, The protocol is standardized, see: http://www.rfc-editor.org/info/rfc7609. I described this and some more protocol details in my patch series overview mail, see for instance: http://marc.info/?l=linux-s390&m=148397751211964&w=2 This description explains the reasons to come up with SMC-R. > * What are the differences between the SMC protocol, the SDP protocol and > the rsockets protocol? How do existing implementations for these protocols > compare to each other from a performance point of view? If no performance > comparison between these protocols is available, shouldn't the performance > of these protocols have been compared with each other before a review of > the SMC driver even started? > * What are the reasons why the SDP driver was never accepted upstream? Do > the arguments why SDP was not accepted upstream also apply to the SMC > driver (SDP = Sockets Direct Protocol)? > * Since SMC has to be selected by specifying AF_SMC, how are users expected > to specify whether AF_INET, AF_INET6 or yet another address family should > be used to set up a connection between SMC > endpoints? The IPv6 support in SMC-R is on our todo-list. > * Is the SMC driver limited to RoCE? Are you aware that the rsockets library > supports multiple transport layers (RoCE, IB and iWARP)? For now, only RoCE is supported. Other transports might be added in the future. > * Since functionality that is similar what the SMC driver provides already > exists in user space (rsockets), why has this functionality been > reimplemented as a kernel driver (SMC)? > > Bart. > Regards, Ursula -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <dce14470-06f4-8da3-6894-cd724eac3447-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>]
* Re: net/smc and the RDMA core [not found] ` <dce14470-06f4-8da3-6894-cd724eac3447-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org> @ 2017-05-02 14:34 ` Doug Ledford 0 siblings, 0 replies; 22+ messages in thread From: Doug Ledford @ 2017-05-02 14:34 UTC (permalink / raw) To: Ursula Braun, Bart Van Assche, hch-jcswGhMUV9g, davem-fT/PcQaiUtIeIZ0/mPfg9Q Cc: netdev-u79uwXL29TY76Z2rM5mHXA, linux-rdma-u79uwXL29TY76Z2rM5mHXA [-- Attachment #1.1: Type: text/plain, Size: 5819 bytes --] On 5/2/2017 8:34 AM, Ursula Braun wrote: > On 05/01/2017 07:29 PM, Bart Van Assche wrote: >> On Mon, 2017-05-01 at 18:33 +0200, Christoph Hellwig wrote: >>> Hi Ursual, hi netdev reviewers, >>> >>> how did the smc protocol manage to get merged without any review >>> on linux-rdma at all? As the results it seems it's very substandard >>> in terms of RDMA API usage, e.g. it neither uses the proper CQ API >>> nor the RDMA R/W API, and other will probably find additional issues >>> as well. >> >> Hello Dave and Ursula, >> >> It seems very rude to me to have merged the SMC protocol driver without >> having involved the linux-rdma community. Anyway, I have the following >> questions for Dave and Ursula: >> * Since the Linux kernel is standards based: where can we find the standard >> that defines the SMC wire protocol? If this protocol has not been >> standardized yet: in what file (other than *.[ch]) in the Linux kernel >> tree has this protocol been documented? > > Hello Bart, > > The protocol is standardized, see: http://www.rfc-editor.org/info/rfc7609. No, SMC-R is *not* standardized. It is informationally publicized. In other words, you guys dumped what you considered your standard out and rfc-editor published it under the "information" status category. To be an Internet Standard requires more work, and additionally requires a period of time for people to comment on this proposal. Given that this is for a linux RDMA communications layer, advertising this RFC on the linux-rdma@ mailing list seems the absolute *minimum* advertising that should be done during the comment phase (at least as relates to the RDMA portion of this protocol, which this protocol is first and foremost an RDMA protocol with only TCP as a control/fallback). It's been 18 months since you published this, and this is the first that it's been brought to the linux-rdma@ mailing list to my knowledge. Anyway, the RFC is informational, not a standard, and as it stands, I'm pretty sure the RDMA community would have a number of comments on it before it were allowed to become a standard, including the fact that it is currently exclusive to RoCEv1 which is, as far as the RDMA community is concerned, a deprecated RoCE mode. Adding a new protocol that only supports this is just simply short sighted. Had we been consulted before this was merged, I have no doubt that we would have had a considerable list of review requirements. But, we are where we are, so the issue is how to move forward. Moving it to staging doesn't seem so inappropriate in this particular case... > I described this and some more protocol details in my patch series > overview mail, see for instance: > http://marc.info/?l=linux-s390&m=148397751211964&w=2 > > This description explains the reasons to come up with SMC-R. > >> * What are the differences between the SMC protocol, the SDP protocol and >> the rsockets protocol? How do existing implementations for these protocols >> compare to each other from a performance point of view? If no performance >> comparison between these protocols is available, shouldn't the performance >> of these protocols have been compared with each other before a review of >> the SMC driver even started? >> * What are the reasons why the SDP driver was never accepted upstream? Do >> the arguments why SDP was not accepted upstream also apply to the SMC >> driver (SDP = Sockets Direct Protocol)? >> * Since SMC has to be selected by specifying AF_SMC, how are users expected >> to specify whether AF_INET, AF_INET6 or yet another address family should >> be used to set up a connection between SMC >> endpoints? > > The IPv6 support in SMC-R is on our todo-list. > >> * Is the SMC driver limited to RoCE? Are you aware that the rsockets library >> supports multiple transport layers (RoCE, IB and iWARP)? > > For now, only RoCE is supported. Other transports might be added in the future. We generally don't allow this type of partial support in RDMA code if we can avoid it. There are means in place in the RDMA subsystem to hide the differences between the different RDMA types and we highly frown on any code that doesn't deal with all of the fabrics. Old code might get grandfathered in if it had good reason for being specific to a fabric, but not new code. Fixing this would have been high on our list of review items. At a minimum supporting iWARP and all flavors of RoCE would have required as these are all Ethernet devices at their heart and should all support the required TCP control channel and such. We *might* have been more lenient on IB and OPA since they don't have a native TCP network device, but since IPoIB works on both, even that isn't really a reason not to support them. The real issue is the much more difficult issue of namespaces and SELinux, which is different between TCP sockets and IB/OPA connections. That would have been the difficult part to get right, but as we haven't investigated it, we really don't know if it would have been solvable in a reasonable fashion. >> * Since functionality that is similar what the SMC driver provides already >> exists in user space (rsockets), why has this functionality been >> reimplemented as a kernel driver (SMC)? >> >> Bart. >> > > Regards, Ursula > > -- > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in > the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> GPG Key ID: B826A3330E572FDD Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 884 bytes --] ^ permalink raw reply [flat|nested] 22+ messages in thread
* RE: net/smc and the RDMA core 2017-05-01 16:33 net/smc and the RDMA core Christoph Hellwig @ 2017-05-01 21:04 ` Steve Wise 2017-05-01 21:04 ` Steve Wise [not found] ` <20170501163311.GA22209-jcswGhMUV9g@public.gmane.org> 2 siblings, 0 replies; 22+ messages in thread From: Steve Wise @ 2017-05-01 21:04 UTC (permalink / raw) To: 'Christoph Hellwig', 'Ursula Braun', 'David S. Miller' Cc: netdev, linux-rdma > > Hi Ursual, hi netdev reviewers, > > how did the smc protocol manage to get merged without any review > on linux-rdma at all? As the results it seems it's very substandard > in terms of RDMA API usage, e.g. it neither uses the proper CQ API > nor the RDMA R/W API, and other will probably find additional issues > as well. > -- And it only supports RoCE (and maybe IB). ^ permalink raw reply [flat|nested] 22+ messages in thread
* RE: net/smc and the RDMA core @ 2017-05-01 21:04 ` Steve Wise 0 siblings, 0 replies; 22+ messages in thread From: Steve Wise @ 2017-05-01 21:04 UTC (permalink / raw) To: 'Christoph Hellwig', 'Ursula Braun', 'David S. Miller' Cc: netdev, linux-rdma > > Hi Ursual, hi netdev reviewers, > > how did the smc protocol manage to get merged without any review > on linux-rdma at all? As the results it seems it's very substandard > in terms of RDMA API usage, e.g. it neither uses the proper CQ API > nor the RDMA R/W API, and other will probably find additional issues > as well. > -- And it only supports RoCE (and maybe IB). ^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <20170501163311.GA22209-jcswGhMUV9g@public.gmane.org>]
* Re: net/smc and the RDMA core [not found] ` <20170501163311.GA22209-jcswGhMUV9g@public.gmane.org> @ 2017-05-02 12:25 ` Ursula Braun [not found] ` <d9214af6-1c6f-9f95-fc00-3e4a316b4f81-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org> 0 siblings, 1 reply; 22+ messages in thread From: Ursula Braun @ 2017-05-02 12:25 UTC (permalink / raw) To: Christoph Hellwig, David S. Miller Cc: netdev-u79uwXL29TY76Z2rM5mHXA, linux-rdma-u79uwXL29TY76Z2rM5mHXA On 05/01/2017 06:33 PM, Christoph Hellwig wrote: > Hi Ursual, hi netdev reviewers, > > how did the smc protocol manage to get merged without any review > on linux-rdma at all? As the results it seems it's very substandard > in terms of RDMA API usage, e.g. it neither uses the proper CQ API > nor the RDMA R/W API, and other will probably find additional issues > as well. > Hi Christoph, We have been posting SMC-R patches on netdev since 2015, there was never any secrecy about it. Still sorry for omitting linux-rdma, will include with future postings from now on. Of course, we are open to any further code reviews, so if you can point out specific issues, we will be happy to work with you to get them addressed! Regards, Ursula -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <d9214af6-1c6f-9f95-fc00-3e4a316b4f81-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>]
* Re: net/smc and the RDMA core [not found] ` <d9214af6-1c6f-9f95-fc00-3e4a316b4f81-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org> @ 2017-05-02 18:39 ` Bart Van Assche [not found] ` <1493750358.2552.13.camel-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org> 0 siblings, 1 reply; 22+ messages in thread From: Bart Van Assche @ 2017-05-02 18:39 UTC (permalink / raw) To: hch-jcswGhMUV9g, davem-fT/PcQaiUtIeIZ0/mPfg9Q, ubraun-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8 Cc: netdev-u79uwXL29TY76Z2rM5mHXA, linux-rdma-u79uwXL29TY76Z2rM5mHXA On Tue, 2017-05-02 at 14:25 +0200, Ursula Braun wrote: > if you can point out specific issues, we will be happy to work with you > to get them addressed! Hello Ursula, My list of issues that I would like to see addressed can be found below. Doug, Christoph and others may have additional inputs. The issues that have not yet been mentioned in other e-mails are: - The SMC driver only supports one RDMA transport type (RoCE v1) but none of the other RDMA transport types (RoCE v2, IB and iWARP). New RDMA drivers should support all RDMA transport types transparently. The traditional approach to support multiple RDMA transport types is by using the RDMA/CM to establish connections. - The implementation of the SMC driver only supports RoCEv1. This is a very unfortunate choice because: - RoCEv1 is not routable and hence is limited to a single Ethernet broadcast domain. - RoCEv1 packets escape a whole bunch of mechanisms that only work for IP packets. Firewalls pass all RoCEv1 packets and switches do not restrict RoCEv1 packets to a single VLAN. This means that if the network configuration is changed after an SMC connection has been set up such that IP communication between the endpoints of an SMC connection is blocked that the SMC RoCEv1 packets will not be blocked by the network equipment of which the configuration has just been changed. - As already mentioned by Christoph, the SMC implementation uses RDMA calls that probably will be deprecated soon (ib_create_cq()) and should use the RDMA R/W API instead of building sge lists itself. Bart.-- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <1493750358.2552.13.camel-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>]
* Re: net/smc and the RDMA core [not found] ` <1493750358.2552.13.camel-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org> @ 2017-05-03 14:40 ` Ursula Braun 2017-05-04 8:43 ` Sagi Grimberg 1 sibling, 0 replies; 22+ messages in thread From: Ursula Braun @ 2017-05-03 14:40 UTC (permalink / raw) To: Bart Van Assche, hch-jcswGhMUV9g, davem-fT/PcQaiUtIeIZ0/mPfg9Q Cc: netdev-u79uwXL29TY76Z2rM5mHXA, linux-rdma-u79uwXL29TY76Z2rM5mHXA On 05/02/2017 08:39 PM, Bart Van Assche wrote: > On Tue, 2017-05-02 at 14:25 +0200, Ursula Braun wrote: >> if you can point out specific issues, we will be happy to work with you >> to get them addressed! > > Hello Ursula, > > My list of issues that I would like to see addressed can be found below. Doug, > Christoph and others may have additional inputs. The issues that have not yet > been mentioned in other e-mails are: > - The SMC driver only supports one RDMA transport type (RoCE v1) but > none of the other RDMA transport types (RoCE v2, IB and iWARP). New > RDMA drivers should support all RDMA transport types transparently. > The traditional approach to support multiple RDMA transport types is > by using the RDMA/CM to establish connections. > - The implementation of the SMC driver only supports RoCEv1. This is > a very unfortunate choice because: > - RoCEv1 is not routable and hence is limited to a single Ethernet > broadcast domain. > - RoCEv1 packets escape a whole bunch of mechanisms that only work > for IP packets. Firewalls pass all RoCEv1 packets and switches > do not restrict RoCEv1 packets to a single VLAN. This means that > if the network configuration is changed after an SMC connection > has been set up such that IP communication between the endpoints > of an SMC connection is blocked that the SMC RoCEv1 packets will > not be blocked by the network equipment of which the configuration > has just been changed. > - As already mentioned by Christoph, the SMC implementation uses RDMA > calls that probably will be deprecated soon (ib_create_cq()) and > should use the RDMA R/W API instead of building sge lists itself. > > Bart. > Thanks for your list, we will take care of it! -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: net/smc and the RDMA core [not found] ` <1493750358.2552.13.camel-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org> 2017-05-03 14:40 ` Ursula Braun @ 2017-05-04 8:43 ` Sagi Grimberg [not found] ` <1b79048f-4495-3840-e7a6-d4fa5a8dfb57-NQWnxTmZq1alnMjI0IkVqw@public.gmane.org> 1 sibling, 1 reply; 22+ messages in thread From: Sagi Grimberg @ 2017-05-04 8:43 UTC (permalink / raw) To: Bart Van Assche, hch-jcswGhMUV9g, davem-fT/PcQaiUtIeIZ0/mPfg9Q, ubraun-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8 Cc: netdev-u79uwXL29TY76Z2rM5mHXA, linux-rdma-u79uwXL29TY76Z2rM5mHXA >> if you can point out specific issues, we will be happy to work with you >> to get them addressed! > > Hello Ursula, > > My list of issues that I would like to see addressed can be found below. Doug, > Christoph and others may have additional inputs. The issues that have not yet > been mentioned in other e-mails are: > - The SMC driver only supports one RDMA transport type (RoCE v1) but > none of the other RDMA transport types (RoCE v2, IB and iWARP). New > RDMA drivers should support all RDMA transport types transparently. > The traditional approach to support multiple RDMA transport types is > by using the RDMA/CM to establish connections. > - The implementation of the SMC driver only supports RoCEv1. This is > a very unfortunate choice because: > - RoCEv1 is not routable and hence is limited to a single Ethernet > broadcast domain. > - RoCEv1 packets escape a whole bunch of mechanisms that only work > for IP packets. Firewalls pass all RoCEv1 packets and switches > do not restrict RoCEv1 packets to a single VLAN. This means that > if the network configuration is changed after an SMC connection > has been set up such that IP communication between the endpoints > of an SMC connection is blocked that the SMC RoCEv1 packets will > not be blocked by the network equipment of which the configuration > has just been changed. > - As already mentioned by Christoph, the SMC implementation uses RDMA > calls that probably will be deprecated soon (ib_create_cq()) and > should use the RDMA R/W API instead of building sge lists itself. I would also suggest that you stop exposing the DMA MR for remote access (at least by default) and use a proper reg_mr operations with a limited lifetime on a properly sized buffer. Also, the cq polling code looks completely wrong, you should really use the RDMA CQ API. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <1b79048f-4495-3840-e7a6-d4fa5a8dfb57-NQWnxTmZq1alnMjI0IkVqw@public.gmane.org>]
* Re: net/smc and the RDMA core [not found] ` <1b79048f-4495-3840-e7a6-d4fa5a8dfb57-NQWnxTmZq1alnMjI0IkVqw@public.gmane.org> @ 2017-05-04 8:48 ` hch-jcswGhMUV9g 2017-05-04 13:08 ` Ursula Braun [not found] ` <20170504084825.GA5399-jcswGhMUV9g@public.gmane.org> 0 siblings, 2 replies; 22+ messages in thread From: hch-jcswGhMUV9g @ 2017-05-04 8:48 UTC (permalink / raw) To: Sagi Grimberg Cc: Bart Van Assche, hch-jcswGhMUV9g, davem-fT/PcQaiUtIeIZ0/mPfg9Q, ubraun-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8, netdev-u79uwXL29TY76Z2rM5mHXA, linux-rdma-u79uwXL29TY76Z2rM5mHXA On Thu, May 04, 2017 at 11:43:50AM +0300, Sagi Grimberg wrote: > I would also suggest that you stop exposing the DMA MR for remote > access (at least by default) and use a proper reg_mr operations with a > limited lifetime on a properly sized buffer. Yes, exposing the default DMA MR is a _major_ security risk. As soon as SMC is enabled this will mean a remote system has full read/write access to the local systems memory. There іs a reason why I removed the ib_get_dma_mr function and replaced it with the IB_PD_UNSAFE_GLOBAL_RKEY key that has _UNSAFE_ in the name and a very long comment explaining why, and I'm really disappointed that we got a driver merged that instead of asking on the relevant list on why a change unexpertong a function it needed happened and instead tried the hard way to keep a security vulnerarbility alive. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: net/smc and the RDMA core 2017-05-04 8:48 ` hch-jcswGhMUV9g @ 2017-05-04 13:08 ` Ursula Braun [not found] ` <efa9bd6d-1df9-952a-7f32-c2ee6bffcae5-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org> 2017-05-04 15:31 ` Jason Gunthorpe [not found] ` <20170504084825.GA5399-jcswGhMUV9g@public.gmane.org> 1 sibling, 2 replies; 22+ messages in thread From: Ursula Braun @ 2017-05-04 13:08 UTC (permalink / raw) To: hch, Sagi Grimberg; +Cc: Bart Van Assche, davem, netdev, linux-rdma On 05/04/2017 10:48 AM, hch@lst.de wrote: > On Thu, May 04, 2017 at 11:43:50AM +0300, Sagi Grimberg wrote: >> I would also suggest that you stop exposing the DMA MR for remote >> access (at least by default) and use a proper reg_mr operations with a >> limited lifetime on a properly sized buffer. > > Yes, exposing the default DMA MR is a _major_ security risk. As soon > as SMC is enabled this will mean a remote system has full read/write > access to the local systems memory. > > There іs a reason why I removed the ib_get_dma_mr function and replaced > it with the IB_PD_UNSAFE_GLOBAL_RKEY key that has _UNSAFE_ in the name > and a very long comment explaining why, and I'm really disappointed that > we got a driver merged that instead of asking on the relevant list on > why a change unexpertong a function it needed happened and instead > tried the hard way to keep a security vulnerarbility alive. > Thanks for pointing out these problems. We will address them. ^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <efa9bd6d-1df9-952a-7f32-c2ee6bffcae5-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>]
* Re: net/smc and the RDMA core [not found] ` <efa9bd6d-1df9-952a-7f32-c2ee6bffcae5-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org> @ 2017-05-04 13:15 ` Leon Romanovsky 0 siblings, 0 replies; 22+ messages in thread From: Leon Romanovsky @ 2017-05-04 13:15 UTC (permalink / raw) To: Ursula Braun Cc: hch-jcswGhMUV9g, Sagi Grimberg, Bart Van Assche, davem-fT/PcQaiUtIeIZ0/mPfg9Q, netdev-u79uwXL29TY76Z2rM5mHXA, linux-rdma-u79uwXL29TY76Z2rM5mHXA [-- Attachment #1: Type: text/plain, Size: 1190 bytes --] On Thu, May 04, 2017 at 03:08:39PM +0200, Ursula Braun wrote: > > > On 05/04/2017 10:48 AM, hch-jcswGhMUV9g@public.gmane.org wrote: > > On Thu, May 04, 2017 at 11:43:50AM +0300, Sagi Grimberg wrote: > >> I would also suggest that you stop exposing the DMA MR for remote > >> access (at least by default) and use a proper reg_mr operations with a > >> limited lifetime on a properly sized buffer. > > > > Yes, exposing the default DMA MR is a _major_ security risk. As soon > > as SMC is enabled this will mean a remote system has full read/write > > access to the local systems memory. > > > > There іs a reason why I removed the ib_get_dma_mr function and replaced > > it with the IB_PD_UNSAFE_GLOBAL_RKEY key that has _UNSAFE_ in the name > > and a very long comment explaining why, and I'm really disappointed that > > we got a driver merged that instead of asking on the relevant list on > > why a change unexpertong a function it needed happened and instead > > tried the hard way to keep a security vulnerarbility alive. > > > Thanks for pointing out these problems. We will address them. > Out of curiosity, when do you plan to address all this? Thanks [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: net/smc and the RDMA core 2017-05-04 13:08 ` Ursula Braun [not found] ` <efa9bd6d-1df9-952a-7f32-c2ee6bffcae5-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org> @ 2017-05-04 15:31 ` Jason Gunthorpe [not found] ` <20170504153155.GB854-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> 1 sibling, 1 reply; 22+ messages in thread From: Jason Gunthorpe @ 2017-05-04 15:31 UTC (permalink / raw) To: Ursula Braun Cc: hch, Sagi Grimberg, Bart Van Assche, davem, netdev, linux-rdma On Thu, May 04, 2017 at 03:08:39PM +0200, Ursula Braun wrote: > > > On 05/04/2017 10:48 AM, hch@lst.de wrote: > > On Thu, May 04, 2017 at 11:43:50AM +0300, Sagi Grimberg wrote: > >> I would also suggest that you stop exposing the DMA MR for remote > >> access (at least by default) and use a proper reg_mr operations with a > >> limited lifetime on a properly sized buffer. > > > > Yes, exposing the default DMA MR is a _major_ security risk. As soon > > as SMC is enabled this will mean a remote system has full read/write > > access to the local systems memory. > > > > There ??s a reason why I removed the ib_get_dma_mr function and replaced > > it with the IB_PD_UNSAFE_GLOBAL_RKEY key that has _UNSAFE_ in the name > > and a very long comment explaining why, and I'm really disappointed that > > we got a driver merged that instead of asking on the relevant list on > > why a change unexpertong a function it needed happened and instead > > tried the hard way to keep a security vulnerarbility alive. > > > Thanks for pointing out these problems. We will address them. So, you've created a huge security hole in the kernel, anyone who loads your smc module is vunerable. What are you going to do *RIGHT NOW* to mitigate this? Jason ^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <20170504153155.GB854-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>]
* Re: net/smc and the RDMA core [not found] ` <20170504153155.GB854-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> @ 2017-05-05 17:06 ` Ursula Braun 2017-05-05 17:10 ` Jason Gunthorpe 0 siblings, 1 reply; 22+ messages in thread From: Ursula Braun @ 2017-05-05 17:06 UTC (permalink / raw) To: Jason Gunthorpe Cc: hch-jcswGhMUV9g, Sagi Grimberg, Bart Van Assche, davem-fT/PcQaiUtIeIZ0/mPfg9Q, netdev-u79uwXL29TY76Z2rM5mHXA, linux-rdma-u79uwXL29TY76Z2rM5mHXA On 05/04/2017 05:31 PM, Jason Gunthorpe wrote: > On Thu, May 04, 2017 at 03:08:39PM +0200, Ursula Braun wrote: >> >> >> On 05/04/2017 10:48 AM, hch-jcswGhMUV9g@public.gmane.org wrote: >>> On Thu, May 04, 2017 at 11:43:50AM +0300, Sagi Grimberg wrote: >>>> I would also suggest that you stop exposing the DMA MR for remote >>>> access (at least by default) and use a proper reg_mr operations with a >>>> limited lifetime on a properly sized buffer. >>> >>> Yes, exposing the default DMA MR is a _major_ security risk. As soon >>> as SMC is enabled this will mean a remote system has full read/write >>> access to the local systems memory. >>> >>> There ??s a reason why I removed the ib_get_dma_mr function and replaced >>> it with the IB_PD_UNSAFE_GLOBAL_RKEY key that has _UNSAFE_ in the name >>> and a very long comment explaining why, and I'm really disappointed that >>> we got a driver merged that instead of asking on the relevant list on >>> why a change unexpertong a function it needed happened and instead >>> tried the hard way to keep a security vulnerarbility alive. >>> >> Thanks for pointing out these problems. We will address them. > > So, you've created a huge security hole in the kernel, anyone who > loads your smc module is vunerable. > > What are you going to do *RIGHT NOW* to mitigate this? > > Jason We do not see that just loading the smc module causes this issue.The security risk starts with the first connection, that actually uses smc. This is only possible if an AF_SMC socket connection is created while the so-called pnet-table is available and offers a mapping between the used Ethernet interface and RoCE device. Such a mapping has to be configured by a user (via a netlink interface) and, thus, is a conscious decision by that user. Nevertheless, thanks for all the valuable feedback; we take this security risk seriously and addressing it is obviously at the top of our list. We're working on this issue right now, and will post patches as soon as possible. Ursula -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: net/smc and the RDMA core 2017-05-05 17:06 ` Ursula Braun @ 2017-05-05 17:10 ` Jason Gunthorpe 2017-05-06 8:25 ` hch 0 siblings, 1 reply; 22+ messages in thread From: Jason Gunthorpe @ 2017-05-05 17:10 UTC (permalink / raw) To: Ursula Braun Cc: hch, Sagi Grimberg, Bart Van Assche, davem, netdev, linux-rdma On Fri, May 05, 2017 at 07:06:56PM +0200, Ursula Braun wrote: > We do not see that just loading the smc module causes this issue.The security > risk starts with the first connection, that actually uses smc. This is only > possible if an AF_SMC socket connection is created while the so-called > pnet-table is available and offers a mapping between the used Ethernet > interface and RoCE device. Such a mapping has to be configured by a user > (via a netlink interface) and, thus, is a conscious decision by that user. At a mimimum this escaltes any local root exploit to a full kernel exploit in the presense of RDMA hardware, so I do not think you should be so dimissive of the impact. I recommend immediately sending a kconfig patch cc'd to stable making SMC require CONFIG_BROKEN so that nobody inadvertantly turns it on. Jason ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: net/smc and the RDMA core 2017-05-05 17:10 ` Jason Gunthorpe @ 2017-05-06 8:25 ` hch 0 siblings, 0 replies; 22+ messages in thread From: hch @ 2017-05-06 8:25 UTC (permalink / raw) To: Jason Gunthorpe Cc: Ursula Braun, hch, Sagi Grimberg, Bart Van Assche, davem, netdev, linux-rdma On Fri, May 05, 2017 at 11:10:17AM -0600, Jason Gunthorpe wrote: > I recommend immediately sending a kconfig patch cc'd to stable making > SMC require CONFIG_BROKEN so that nobody inadvertantly turns it on. Yes, I'll send the patch. ^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <20170504084825.GA5399-jcswGhMUV9g@public.gmane.org>]
* Re: net/smc and the RDMA core [not found] ` <20170504084825.GA5399-jcswGhMUV9g@public.gmane.org> @ 2017-05-11 16:50 ` Ursula Braun [not found] ` <869d9fb6-0d83-5f57-f8e4-5c1ee7477b94-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org> 0 siblings, 1 reply; 22+ messages in thread From: Ursula Braun @ 2017-05-11 16:50 UTC (permalink / raw) To: hch-jcswGhMUV9g, Sagi Grimberg Cc: Bart Van Assche, davem-fT/PcQaiUtIeIZ0/mPfg9Q, netdev-u79uwXL29TY76Z2rM5mHXA, linux-rdma-u79uwXL29TY76Z2rM5mHXA On 05/04/2017 10:48 AM, hch-jcswGhMUV9g@public.gmane.org wrote: > On Thu, May 04, 2017 at 11:43:50AM +0300, Sagi Grimberg wrote: >> I would also suggest that you stop exposing the DMA MR for remote >> access (at least by default) and use a proper reg_mr operations with a >> limited lifetime on a properly sized buffer. > > Yes, exposing the default DMA MR is a _major_ security risk. As soon > as SMC is enabled this will mean a remote system has full read/write > access to the local systems memory. > > There іs a reason why I removed the ib_get_dma_mr function and replaced > it with the IB_PD_UNSAFE_GLOBAL_RKEY key that has _UNSAFE_ in the name > and a very long comment explaining why, and I'm really disappointed that > we got a driver merged that instead of asking on the relevant list on > why a change unexpertong a function it needed happened and instead > tried the hard way to keep a security vulnerarbility alive. > Please consider the following patch to make users aware of the security implications through existing mechanisms. Work on proper usage of reg_mr operations is underway, respective patches will follow as soon as possible. --- net/smc/smc_core.c | 16 +++------------- net/smc/smc_ib.c | 21 ++------------------- net/smc/smc_ib.h | 2 -- 3 files changed, 5 insertions(+), 34 deletions(-) diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c index d52a2ee..6ec3d9a 100644 --- a/net/smc/smc_core.c +++ b/net/smc/smc_core.c @@ -613,19 +613,8 @@ int smc_rmb_create(struct smc_sock *smc) rmb_desc = NULL; continue; /* if mapping failed, try smaller one */ } - rc = smc_ib_get_memory_region(lgr->lnk[SMC_SINGLE_LINK].roce_pd, - IB_ACCESS_REMOTE_WRITE | - IB_ACCESS_LOCAL_WRITE, - &rmb_desc->mr_rx[SMC_SINGLE_LINK]); - if (rc) { - smc_ib_buf_unmap(lgr->lnk[SMC_SINGLE_LINK].smcibdev, - tmp_bufsize, rmb_desc, - DMA_FROM_DEVICE); - kfree(rmb_desc->cpu_addr); - kfree(rmb_desc); - rmb_desc = NULL; - continue; - } + rmb_desc->mr_rx[SMC_SINGLE_LINK] = + lgr->lnk[SMC_SINGLE_LINK].roce_pd->__internal_mr; rmb_desc->used = 1; write_lock_bh(&lgr->rmbs_lock); list_add(&rmb_desc->list, @@ -668,6 +657,7 @@ int smc_rmb_rtoken_handling(struct smc_connection *conn, for (i = 0; i < SMC_RMBS_PER_LGR_MAX; i++) { if ((lgr->rtokens[i][SMC_SINGLE_LINK].rkey == rkey) && + (lgr->rtokens[i][SMC_SINGLE_LINK].dma_addr == dma_addr) && test_bit(i, lgr->rtokens_used_mask)) { conn->rtoken_idx = i; return 0; diff --git a/net/smc/smc_ib.c b/net/smc/smc_ib.c index 3d0d91c..6af9ebf 100644 --- a/net/smc/smc_ib.c +++ b/net/smc/smc_ib.c @@ -37,24 +37,6 @@ u8 local_systemid[SMC_SYSTEMID_LEN] = SMC_LOCAL_SYSTEMID_RESET; /* unique system * identifier */ -int smc_ib_get_memory_region(struct ib_pd *pd, int access_flags, - struct ib_mr **mr) -{ - int rc; - - if (*mr) - return 0; /* already done */ - - /* obtain unique key - - * next invocation of get_dma_mr returns a different key! - */ - *mr = pd->device->get_dma_mr(pd, access_flags); - rc = PTR_ERR_OR_ZERO(*mr); - if (IS_ERR(*mr)) - *mr = NULL; - return rc; -} - static int smc_ib_modify_qp_init(struct smc_link *lnk) { struct ib_qp_attr qp_attr; @@ -211,7 +193,8 @@ int smc_ib_create_protection_domain(struct smc_link *lnk) { int rc; - lnk->roce_pd = ib_alloc_pd(lnk->smcibdev->ibdev, 0); + lnk->roce_pd = ib_alloc_pd(lnk->smcibdev->ibdev, + IB_PD_UNSAFE_GLOBAL_RKEY); rc = PTR_ERR_OR_ZERO(lnk->roce_pd); if (IS_ERR(lnk->roce_pd)) lnk->roce_pd = NULL; diff --git a/net/smc/smc_ib.h b/net/smc/smc_ib.h index e5bb3c9..55c529b 100644 --- a/net/smc/smc_ib.h +++ b/net/smc/smc_ib.h @@ -60,8 +60,6 @@ void smc_ib_dealloc_protection_domain(struct smc_link *lnk); int smc_ib_create_protection_domain(struct smc_link *lnk); void smc_ib_destroy_queue_pair(struct smc_link *lnk); int smc_ib_create_queue_pair(struct smc_link *lnk); -int smc_ib_get_memory_region(struct ib_pd *pd, int access_flags, - struct ib_mr **mr); int smc_ib_ready_link(struct smc_link *lnk); int smc_ib_modify_qp_rts(struct smc_link *lnk); int smc_ib_modify_qp_reset(struct smc_link *lnk); -- Do you think it is worth the effort for this intermediate patch? -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply related [flat|nested] 22+ messages in thread
[parent not found: <869d9fb6-0d83-5f57-f8e4-5c1ee7477b94-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>]
* Re: net/smc and the RDMA core [not found] ` <869d9fb6-0d83-5f57-f8e4-5c1ee7477b94-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org> @ 2017-05-11 16:56 ` hch-jcswGhMUV9g 0 siblings, 0 replies; 22+ messages in thread From: hch-jcswGhMUV9g @ 2017-05-11 16:56 UTC (permalink / raw) To: Ursula Braun Cc: hch-jcswGhMUV9g, Sagi Grimberg, Bart Van Assche, davem-fT/PcQaiUtIeIZ0/mPfg9Q, netdev-u79uwXL29TY76Z2rM5mHXA, linux-rdma-u79uwXL29TY76Z2rM5mHXA On Thu, May 11, 2017 at 06:50:04PM +0200, Ursula Braun wrote: > Please consider the following patch to make users aware of the > security implications through existing mechanisms. Any such patch would be in addition to the BROKEN marker until there is an actual alternative. > + rmb_desc->mr_rx[SMC_SINGLE_LINK] = > + lgr->lnk[SMC_SINGLE_LINK].roce_pd->__internal_mr; And it's named __internal_mr for reason. The right interface to use the unsafe rkey is to use pd->unsafe_global_rkey. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2017-05-11 16:56 UTC | newest] Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-05-01 16:33 net/smc and the RDMA core Christoph Hellwig 2017-05-01 17:29 ` Bart Van Assche [not found] ` <1493659776.2665.7.camel-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org> 2017-05-01 17:55 ` Parav Pandit [not found] ` <HE1PR0502MB30048AFD086C4B0D535BFC52D1140-692Kmc8YnlL9PhveBwpv4cDSnupUy6xnnBOFsp37pqbUKgpGm//BTAC/G2K4zDHf@public.gmane.org> 2017-05-02 12:41 ` Ursula Braun 2017-05-02 15:37 ` Bart Van Assche 2017-05-02 12:34 ` Ursula Braun [not found] ` <dce14470-06f4-8da3-6894-cd724eac3447-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org> 2017-05-02 14:34 ` Doug Ledford 2017-05-01 21:04 ` Steve Wise 2017-05-01 21:04 ` Steve Wise [not found] ` <20170501163311.GA22209-jcswGhMUV9g@public.gmane.org> 2017-05-02 12:25 ` Ursula Braun [not found] ` <d9214af6-1c6f-9f95-fc00-3e4a316b4f81-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org> 2017-05-02 18:39 ` Bart Van Assche [not found] ` <1493750358.2552.13.camel-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org> 2017-05-03 14:40 ` Ursula Braun 2017-05-04 8:43 ` Sagi Grimberg [not found] ` <1b79048f-4495-3840-e7a6-d4fa5a8dfb57-NQWnxTmZq1alnMjI0IkVqw@public.gmane.org> 2017-05-04 8:48 ` hch-jcswGhMUV9g 2017-05-04 13:08 ` Ursula Braun [not found] ` <efa9bd6d-1df9-952a-7f32-c2ee6bffcae5-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org> 2017-05-04 13:15 ` Leon Romanovsky 2017-05-04 15:31 ` Jason Gunthorpe [not found] ` <20170504153155.GB854-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> 2017-05-05 17:06 ` Ursula Braun 2017-05-05 17:10 ` Jason Gunthorpe 2017-05-06 8:25 ` hch [not found] ` <20170504084825.GA5399-jcswGhMUV9g@public.gmane.org> 2017-05-11 16:50 ` Ursula Braun [not found] ` <869d9fb6-0d83-5f57-f8e4-5c1ee7477b94-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org> 2017-05-11 16:56 ` hch-jcswGhMUV9g
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.