Date:   Tue, 2 May 2017 10:34:18 -0400
From:   Doug Ledford <dledford@...hat.com>
To:     Ursula Braun <ubraun@...ux.vnet.ibm.com>,
        Bart Van Assche <Bart.VanAssche@...disk.com>,
        "hch@....de" <hch@....de>,
        "davem@...emloft.net" <davem@...emloft.net>
Cc:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>
Subject: 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
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@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


-- 
Doug Ledford <dledford@...hat.com>
    GPG Key ID: B826A3330E572FDD
    Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD



Download attachment "signature.asc" of type "application/pgp-signature" (885 bytes)

Powered by blists - more mailing lists