On Thu, 2017-02-23 at 15:11 -0500, J. Bruce Fields wrote: > On Thu, Feb 23, 2017 at 03:06:25PM -0500, Tom Talpey wrote: > > On 2/23/2017 3:00 PM, Jeff Layton wrote: > > > On Thu, 2017-02-23 at 14:42 -0500, Tom Talpey wrote: > > > > On 2/23/2017 12:03 PM, Jeff Layton wrote: > > > > > Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx> > > > > > --- > > > > > include/linux/sunrpc/svc_xprt.h | 1 + > > > > > net/sunrpc/svcsock.c | 1 + > > > > > net/sunrpc/xprtrdma/svc_rdma_transport.c | 2 ++ > > > > > > > > There's a possibly-important detail here. Not all RDMA transports have > > > > "IETF-approved congestion control", for example, RoCEv1. However, iWARP > > > > and (arguably) RoCEv2 do. On the other hand, as a nonroutable protocol, > > > > RoCEv1 may not fall under this restriction. > > > > > > > > Net-net, inspecting only the RDMA attribute of the transport may be > > > > insufficient here. > > > > > > > > It could be argued however that the xprtrdma layer, with its rpcrdma > > > > crediting, provides such congestion. But that needs to be made > > > > explicit, and perhaps, discussed in IETF. Initially, I think it would > > > > be important to flag this as a point for the future. For now, it may > > > > be best to flag RoCEv1 as not supporting congestion. > > > > > > > > Tom. > > > > > > > > > > (cc'ing Chuck and the linux-rdma list) > > > > > > Thanks Tom, that's very interesting. > > > > > > Not being well versed in the xprtrdma layer, what condition should we > > > use here to set the flag? git grep shows that the string "ROCEV1" only > > > shows up in the bxnt_en driver. Is there some way to determine this > > > generically for any given RDMA driver? > > > > I would not code RoCEv1 as an exception, I would code iWARP and RoCEv2 > > as the only eligible ones. There are any number of other possibilities, > > none of which should be automatically flagged as congestion-controlled. > > > > I'm also not sure I'm comfortable with hardcoding such a list into RPC. > > But it may be the best you can do for now. Chuck, are you aware of a > > verbs interface to obtain the RDMA transport type? > > If this gets too complicated--we've been allowing NFSv4/UDP for years, > letting this one (arguable?) exception through in RDMA a little longer > won't kill us. > That's my feeling too. This is still an improvement over the status quo, and hopefully anyone with RDMA hardware will have a bit more clue as to whether it can properly support v4. We can always further restrict when rdma_create_xprt sets the flag in a later patch if we figure out some way to determine this generically. I will plan to add a comment that we're setting this RDMA svc_xprt universally even though it may not always be true. > (And if we really shouldn't be doing NFSv4 over some RDMA transports--is > it worth supporting them at all, if the only support we can get is > NFSv3-only?) > I'd be inclined to leave them working and just deny the use of v4 on such transports. -- Jeff Layton <jlayton@xxxxxxxxxx> -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html