On Mon, 2018-12-24 at 11:59 +0300, Vasily Averin wrote: > On 12/24/18 11:21 AM, Trond Myklebust wrote: > > On Mon, 2018-12-24 at 09:05 +0300, Vasily Averin wrote: > > > On 12/24/18 8:51 AM, Vasily Averin wrote: > > > > On 12/24/18 2:56 AM, Trond Myklebust wrote: > > > > > On Sat, 2018-12-22 at 20:46 +0300, Vasily Averin wrote: > > > > > > On 12/21/18 4:00 AM, bfields@xxxxxxxxxxxx wrote: > > > > > > > On Tue, Dec 18, 2018 at 02:55:15PM +0000, Trond Myklebust > > > > > > > wrote: > > > > > > > > No. We don't care about xpt_flags for the back channel > > > > > > > > because > > > > > > > > there is > > > > > > > > no "server transport". The actual transport is stored > > > > > > > > in > > > > > > > > the > > > > > > > > 'struct > > > > > > > > rpc_rqst', and is the struct rpc_xprt corresponding to > > > > > > > > the > > > > > > > > client > > > > > > > > socket or RDMA channel. > > > > > > > > > > > > > > > > IOW: All we really need in svc_process_common() is to > > > > > > > > be > > > > > > > > able to > > > > > > > > run > > > > > > > > rqstp->rq_xprt->xpt_ops->xpo_prep_reply_hdr(), and that > > > > > > > > can > > > > > > > > be > > > > > > > > passed > > > > > > > > either as a pointer to the struct svc_xprt_ops itself. > > > > > > > > > > > > > > For what it's worth, I'd rather get rid of that op--it's > > > > > > > an > > > > > > > awfully > > > > > > > roundabout way just to do "svc_putnl(resv, 0);" in the > > > > > > > tcp > > > > > > > case. > > > > > > > > > > > > Do you mean that svc_create_xprt(serv, "tcp-bc", ...) was > > > > > > used > > > > > > ONLY > > > > > > to call > > > > > > svc_tcp_prep_reply_hdr() in svc_process_common() ? > > > > > > And according call for rdma-bc does nothing useful at all? > > > > > > > > > > > > I've just tried to remove svc_create_xprt() from > > > > > > xs_tcp_bc_up() > > > > > > and > > > > > > just > > > > > > provide pointer to svc_tcp_prep_reply_hdr() > > > > > > in svc_process_common() > > > > > > via per-netns sunrpc_net -- and seems it was enough, my > > > > > > testcase > > > > > > worked correctly. > > > > > > > > > > I don't see how that function is related to net namespaces. > > > > > As > > > > > far as I > > > > > can tell, it only signals whether or not the type of > > > > > transport > > > > > uses the > > > > > TCP record marking scheme. > > > > > > > > We need to know which kind of transport is used in specified > > > > net > > > > namespace, > > > > for example init_ns can use RDMA transport and netns "second" > > > > can > > > > use > > > > TCP transport at the same time. > > > > If you do not like an idea to use function pointer as a mark -- > > > > ok > > > > I can save only some boolean flag on sunrpc_net, check it in > > > > svc_process_common() > > > > and if it is set -- call svc_tcp_prep_reply_hdr() directly. > > > > I'm not against the idea of using a function pointer, but I'm > > saying > > that the transport is not unique per-netns. Instead, the transport > > is > > usually per NFS mount, but you can always retrieve a pointer to it > > directly in bc_svc_process() from req->rq_xprt. > > You're right, I was wrong because I was focused on creation of fake > transport svc_xprt. > Yes, we cannot use per-netns pointer here. > > > > moreover, I can do not change sunrpc_net at all, > > > I can check in bc_svc_common() which transport uses incoming > > > svc_req > > > and provide such flag as new parameter to svc_process_common(). > > > > The function or flag used by bc_svc_common() could be added to req- > > > rq_xprt->ops as another 'bc_' field and then passed to > > svc_process_common() as the parameter. > > Can I just check rqstp->rq_prot ? It is inherited from incoming > svc_req, > and it seems it enough to check its propo, it isn't? > > svc_process_common() > ... > /* Setup reply header */ > if (rqstp->rq_prot == IPPROTO_TCP) > svc_tcp_prep_reply_hdr(rqstp); Yes. In these days with retpoline slowing down all indirect function calls, then the above is probably the better solution. -- Trond Myklebust Linux NFS client maintainer, Hammerspace trond.myklebust@xxxxxxxxxxxxxxx