[PATCH V3 net 0/1] net/smc and the RDMA core
From: Ursula Braun Hi Dave, as requested, here is V3 of the smc-patch with an updated commit log. V3: update commit log V2: do not use _internal_mr V1: switch to usage of IB_PD_UNSAFE_GLOBAL_RKEY Kind regards, Ursula Ursula Braun (1): smc: switch to usage of IB_PD_UNSAFE_GLOBAL_RKEY net/smc/smc_clc.c | 4 ++-- net/smc/smc_core.c | 16 +++- net/smc/smc_core.h | 2 +- net/smc/smc_ib.c | 21 ++--- net/smc/smc_ib.h | 2 -- 5 files changed, 8 insertions(+), 37 deletions(-) -- 2.10.2
[PATCH V2 net 0/1] net/smc and the RDMA core
From: Ursula Braun Hi Dave, yesterday I included a patch proposal into a response to Christoph Hellwig, which is now already seen here: http://patchwork.ozlabs.org/patch/761250/ Christoph suggested an additional improvement not to use __internal_mr. Thus I come up with this improved version V2. Kind regards, Ursula Ursula Braun (1): smc: switch to usage of IB_PD_UNSAFE_GLOBAL_RKEY net/smc/smc_clc.c | 4 ++-- net/smc/smc_core.c | 16 +++- net/smc/smc_core.h | 2 +- net/smc/smc_ib.c | 21 ++--- net/smc/smc_ib.h | 2 -- 5 files changed, 8 insertions(+), 37 deletions(-) -- 2.10.2
Re: net/smc and the RDMA core
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.
Re: net/smc and the RDMA core
On 05/04/2017 10:48 AM, h...@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. > 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 *ln
Re: net/smc and the RDMA core
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.
Re: net/smc and the RDMA core
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
Re: net/smc and the RDMA core
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, h...@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 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
Re: net/smc and the RDMA core
On Thu, May 04, 2017 at 03:08:39PM +0200, Ursula Braun wrote: > > > On 05/04/2017 10:48 AM, h...@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
Re: net/smc and the RDMA core
On Thu, May 04, 2017 at 03:08:39PM +0200, Ursula Braun wrote: > > > On 05/04/2017 10:48 AM, h...@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. > Out of curiosity, when do you plan to address all this? Thanks signature.asc Description: PGP signature
Re: net/smc and the RDMA core
On 05/04/2017 10:48 AM, h...@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.
Re: net/smc and the RDMA core
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.
Re: net/smc and the RDMA core
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.
Re: net/smc and the RDMA core
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!
Re: net/smc and the RDMA core
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.
Re: net/smc and the RDMA core
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.
Re: net/smc and the RDMA core
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 real
Re: net/smc and the RDMA core
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-ow...@vger.kernel.org [mailto:linux-rdma- >> ow...@vger.kernel.org] On Behalf Of Bart Van Assche >> Sent: Monday, May 1, 2017 12:30 PM >> To: h...@lst.de; da...@davemloft.net; ubr...@linux.vnet.ibm.com >> Cc: netdev@vger.kernel.org; linux-r...@vger.kernel.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 majord...@vger.kernel.org More majordomo info at >> http://vger.kernel.org/majordomo-info.html >
Re: net/smc and the RDMA core
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
Re: net/smc and the RDMA core
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
RE: net/smc and the RDMA core
> > 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).
RE: net/smc and the RDMA core
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-ow...@vger.kernel.org [mailto:linux-rdma- > ow...@vger.kernel.org] On Behalf Of Bart Van Assche > Sent: Monday, May 1, 2017 12:30 PM > To: h...@lst.de; da...@davemloft.net; ubr...@linux.vnet.ibm.com > Cc: netdev@vger.kernel.org; linux-r...@vger.kernel.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 majord...@vger.kernel.org More majordomo info at > http://vger.kernel.org/majordomo-info.html
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.
net/smc and the RDMA core
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.