> On Nov 1, 2015, at 4:53 PM, Peter Rosin <peda@xxxxxxxxxxxxxx> wrote: > > > On 2015-11-01 20:26, Chuck Lever wrote: >>> On Nov 1, 2015, at 12:57 PM, Peter Rosin <peda@xxxxxxxxxxxxxx> wrote: >>> >>> >>> On 2015-11-01 11:51, Andreas Radke wrote: >>>> Am Sat, 31 Oct 2015 15:51:54 -0400 >>>> schrieb Steve Dickson <SteveD@xxxxxxxxxx>: >>>> >>>>> Hello, >>>>> >>>>> The 1.0.1 version of libtirpc has just been release. >>>>> >>>>> In this release the SONAME has been changed to 3.0.0 to >>>>> reflect a number of changes in the API. Those changes >>>>> were needed to make the Linux version of libtirpc >>>>> more compatible with other implementations >>>> This break rpcbind recompilation: >>>> >>>> src/rpcb_svc_com.c: In function 'handle_reply': >>>> src/rpcb_svc_com.c:1298:6: error: 'SVCXPRT {aka struct __rpc_svcxprt}' >>>> has no member named 'xp_auth' xprt->xp_auth = &svc_auth_none; >>>> ^ >>>> In file included from /usr/include/tirpc/rpc/rpc.h:62:0, >>>> from src/rpcb_svc_com.c:48: >>>> src/rpcb_svc_com.c:1300:22: error: 'SVCXPRT {aka struct __rpc_svcxprt}' >>>> has no member named 'xp_auth' SVCAUTH_DESTROY(xprt->xp_auth); >>>> ^ >>>> /usr/include/tirpc/rpc/svc_auth.h:63:7: note: in definition of macro >>>> 'SVCAUTH_DESTROY' ((*((auth)->svc_ah_ops->svc_ah_destroy))(auth)) >>>> ^ >>>> src/rpcb_svc_com.c:1300:22: error: 'SVCXPRT {aka struct __rpc_svcxprt}' >>>> has no member named 'xp_auth' SVCAUTH_DESTROY(xprt->xp_auth); >>>> ^ >>>> /usr/include/tirpc/rpc/svc_auth.h:63:43: note: in definition of macro >>>> 'SVCAUTH_DESTROY' ((*((auth)->svc_ah_ops->svc_ah_destroy))(auth)) >>>> ^ >>>> src/rpcb_svc_com.c:1301:6: error: 'SVCXPRT {aka struct __rpc_svcxprt}' >>>> has no member named 'xp_auth' xprt->xp_auth = NULL; >>>> ^ >>>> Makefile:481: recipe for target 'src/rpcb_svc_com.o' failed >>>> make: *** [src/rpcb_svc_com.o] Error 1 >>>> make: *** Waiting for unfinished jobs.... >>>> ==> ERROR: A failure occurred in build(). >>>> >>>> Do you have a fix? >>>> >>> Should be as simple as (not even compile-tested): >>> >>> diff --git a/src/rpcb_svc_com.c b/src/rpcb_svc_com.c >>> index 4ae93f1..38f163f 100644 >>> --- a/src/rpcb_svc_com.c >>> +++ b/src/rpcb_svc_com.c >>> @@ -1295,10 +1295,8 @@ handle_reply(int fd, SVCXPRT *xprt) >>> a.rmt_localvers = fi->versnum; >>> >>> xprt_set_caller(xprt, fi); >>> - xprt->xp_auth = &svc_auth_none; >>> + SVC_XP_AUTH(xprt) = svc_auth_none; >>> svc_sendreply(xprt, (xdrproc_t) xdr_rmtcall_result, (char *) &a); >>> - SVCAUTH_DESTROY(xprt->xp_auth); >>> - xprt->xp_auth = NULL; >>> done: >>> if (buffer) >>> free(buffer); >>> >>> But that breaks compatibility with earlier libtirpc of course… >>> >> #if defined(SVC_XP_AUTH) >> SVC_XP_AUTH(xprt) = svc_auth_none; >> #else >> >> . . . >> >> #endif >> >> But I wonder if that’s even necessary now. See rpcbind >> commit 86036582c001. >> > Yes, it is fishy to clobber the server auth stuff, so it is probably best to just zap > the svc_auth_none assignment altogether. However, the core initializes the > server auth to svc_auth_none at the beginning of handling each separate call, > so if you somehow use a xprt to send replies before it has taken a call (is that > even possible?), there will be no server auth. In that very dubious case, the > assignment is essential. > > I have not looked at the rpcbind code in any depth whatsoever and don't know > anything about the semantics of this "handle_reply" function. But, since it is > a "reply", there will presumably have been a preceding "call", presumably on > the same transport, in which case the server auth have been initialized. So, it > is safe to drop the svc_auth_none assignment. Presumably. :-) The original 2012 rpcbind fix smells a little like a workaround for a libtirpc bug. It would be nice if there was a unit test somewhere to confirm that setting xp_auth is no longer needed with the current libtirpc (or help us ferret out the libtirpc bug if it still exists). -- Chuck Lever chucklever@xxxxxxxxx -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html