clnt_create: Restore using reserve ports for client connections

PATCH In review Steve Dickson 2 participants 4 messages Apr 11, 2018 – Apr 12, 2018

Subsystems (2)

NFS client NFS server

lore

Share & tools

lore

Steve Dickson

author

2018-04-11 15:08 UTC #0 lore raw

PATCH 1/1 clnt_create: Restore using reserve ports for client connections

Commit 46e04a73 changed clnt_create(3) not to use reserve ports when binding a connection to a server.

For certain legacy apps, like some of the NIS commands, the client has to used reserve port to be able to communicate with the server.

So the use of reserve ports in clnt_create(3) has been restored.

Signed-off-by Steve Dickson <[email protected]>

---
 src/clnt_generic.c | 3 +--
 src/rpc_soc.c      | 3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

src/clnt_generic.c +1 -2

index e5a314f..774292b 100644
@@ -341,8 +341,7 @@ clnt_tli_create(int fd, const struct netconfig *nconf,
 		servtype = nconf->nc_semantics;
 		if (!__rpc_fd2sockinfo(fd, &si))
 			goto err;
-		if (__binddynport(fd) == -1)
-			goto err;
+		bindresvport(fd, NULL);
 	} else {
 		if (!__rpc_fd2sockinfo(fd, &si))
 			goto err;

src/rpc_soc.c +1 -2

index af6c482..f32a27c 100644
@@ -147,8 +147,7 @@ clnt_com_create(raddr, prog, vers, sockp, sendsz, recvsz, tp, flags)
 	bindaddr.maxlen = bindaddr.len =  sizeof (struct sockaddr_in);
 	bindaddr.buf = raddr;
 
-	if (__binddynport(fd) == -1)
-		goto err;
+	bindresvport(fd, NULL);
 	cl = clnt_tli_create(fd, nconf, &bindaddr, prog, vers,
 				sendsz, recvsz);
 	if (cl) {

Signature · 1 line

2.14.3

Chuck Lever

2018-04-11 15:21 UTC #1 lore raw

Hi Steve-

Quoted 9 lines · Steve Dickson

=20
Commit 46e04a73 changed clnt_create(3) not to use reserve ports when binding a connection to a server.
=20
For certain legacy apps, like some of the NIS commands, the client has to used reserve port to be able to communicate with the server.

This is still not clear.

The problem is not that the RPC client has to use a reserved port, it's that the client implementations currently assume that clnt_create(3) will allocate a reserved port for them, which is undocumented API behavior.

One more comment below.

src/clnt_generic.c clnt_tli_create(int fd, const struct netconfig =

Quoted 14 lines

So the use of reserve ports in clnt_create(3) has been restored.
=20
Signed-off-by Steve Dickson <[email protected]>
---
src/clnt_generic.c | 3 +--
src/rpc_soc.c | 3 +--
2 files changed, 2 insertions(+), 4 deletions(-)
=20
src/clnt_generic.c +0 -0
index e5a314f..774292b 100644
@@ -341,8 +341,7 @@ clnt_tli_create(int fd, const struct netconfig =

*nconf,

src/clnt_generic.c clnt_tli_create(int fd, const struct netconfig *nconf, · line 344

Quoted 6 lines

src/clnt_generic.c +1 -2

 		servtype =3D nconf->nc_semantics;
 		if (!__rpc_fd2sockinfo(fd, &si))
 			goto err;
-		if (__binddynport(fd) =3D=3D -1)
-			goto err;
+		bindresvport(fd, NULL);

If this is the last __binddynport call site in clnt_generic.c, you can remove its forward declaration (line 50 in my copy of the source).

src/rpc_soc.c clnt_com_create(raddr, prog, vers, sockp, sendsz, =

Quoted 8 lines

src/clnt_generic.c +0 -0

 	} else {
 		if (!__rpc_fd2sockinfo(fd, &si))
 			goto err;

src/rpc_soc.c +0 -0

index af6c482..f32a27c 100644
@@ -147,8 +147,7 @@ clnt_com_create(raddr, prog, vers, sockp, sendsz, =

recvsz, tp, flags)

src/rpc_soc.c clnt_com_create(raddr, prog, vers, sockp, sendsz, = · line 147

Quoted 1 line

src/rpc_soc.c +0 -0

 	bindaddr.maxlen =3D bindaddr.len =3D  sizeof (struct =

sockaddr_in);

Quoted 8 lines

src/rpc_soc.c +0 -0

 	bindaddr.buf =3D raddr;

=20

src/rpc_soc.c +1 -2

-	if (__binddynport(fd) =3D=3D -1)
-		goto err;
+	bindresvport(fd, NULL);

	cl =3D clnt_tli_create(fd, nconf, &bindaddr, prog, vers,
				sendsz, recvsz);
	if (cl) {

Quoted 5 lines

--=20
2.14.3
=20
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" =

Quoted 2 lines

the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

--
Chuck Lever

Steve Dickson

author

2018-04-11 15:52 UTC #2 lore raw

Quoted 12 lines · Chuck Lever

Hi Steve-
Quoted 9 lines · Steve Dickson
Commit 46e04a73 changed clnt_create(3) not to
use reserve ports when binding a connection to
a server.
For certain legacy apps, like some of the NIS commands, the client has to used reserve port to be able to communicate with the server.

Quoted 6 lines

This is still not clear.
The problem is not that the RPC client has to use a reserved port, it's that the client implementations currently assume that clnt_create(3) will allocate a reserved port for them, which is undocumented API behavior.

An assumption that that has been valid for the last 20yrs so that assumption is part of the API... IMHO...

I'm going to leave as is...

Quoted 13 lines

One more comment below.
Quoted 9 lines
So the use of reserve ports in clnt_create(3)
has been restored.
Signed-off-by Steve Dickson <[email protected]>
---
src/clnt_generic.c | 3 +--
src/rpc_soc.c | 3 +--
2 files changed, 2 insertions(+), 4 deletions(-)

Quoted 15 lines

src/clnt_generic.c clnt_tli_create(int fd, const struct netconfig *nconf, · line 344
Quoted 11 lines
src/clnt_generic.c +1 -2
index e5a314f..774292b 100644
@@ -341,8 +341,7 @@ clnt_tli_create(int fd, const struct netconfig *nconf,
 		servtype = nconf->nc_semantics;
 		if (!__rpc_fd2sockinfo(fd, &si))
 			goto err;
-		if (__binddynport(fd) == -1)
-			goto err;
+		bindresvport(fd, NULL);
If this is the last __binddynport call site in clnt_generic.c, you can remove its forward declaration (line 50 in my copy of the source).

Yeah there is a couple of those... already cleaned up...

Thanks..

steved.

Quoted 32 lines

Quoted 26 lines

src/clnt_generic.c +0 -0

 	} else {
 		if (!__rpc_fd2sockinfo(fd, &si))
 			goto err;

src/rpc_soc.c +1 -2

index af6c482..f32a27c 100644
@@ -147,8 +147,7 @@ clnt_com_create(raddr, prog, vers, sockp, sendsz, recvsz, tp, flags)
 	bindaddr.maxlen = bindaddr.len =  sizeof (struct sockaddr_in);
 	bindaddr.buf = raddr;
 
-	if (__binddynport(fd) == -1)
-		goto err;
+	bindresvport(fd, NULL);
 	cl = clnt_tli_create(fd, nconf, &bindaddr, prog, vers,
 				sendsz, recvsz);
 	if (cl) {

Signature · 6 lines

2.14.3

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
Chuck Lever

Steve Dickson

author

2018-04-12 14:26 UTC #3 lore raw

In reply to Steve Dickson

Quoted 4 lines · Steve Dickson

Commit 46e04a73 changed clnt_create(3) not to use reserve ports when binding a connection to a server.

Quoted 8 lines

For certain legacy apps, like some of the NIS commands, the client has to used reserve port to be able to communicate with the server.
So the use of reserve ports in clnt_create(3) has been restored.
Signed-off-by Steve Dickson <[email protected]>

Committed...

steved.

Quoted 34 lines

---
 src/clnt_generic.c | 3 +--
 src/rpc_soc.c      | 3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

src/clnt_generic.c +1 -2

index e5a314f..774292b 100644
@@ -341,8 +341,7 @@ clnt_tli_create(int fd, const struct netconfig *nconf,
 		servtype = nconf->nc_semantics;
 		if (!__rpc_fd2sockinfo(fd, &si))
 			goto err;
-		if (__binddynport(fd) == -1)
-			goto err;
+		bindresvport(fd, NULL);
 	} else {
 		if (!__rpc_fd2sockinfo(fd, &si))
 			goto err;

src/rpc_soc.c +1 -2

index af6c482..f32a27c 100644
@@ -147,8 +147,7 @@ clnt_com_create(raddr, prog, vers, sockp, sendsz, recvsz, tp, flags)
 	bindaddr.maxlen = bindaddr.len =  sizeof (struct sockaddr_in);
 	bindaddr.buf = raddr;
 
-	if (__binddynport(fd) == -1)
-		goto err;
+	bindresvport(fd, NULL);
 	cl = clnt_tli_create(fd, nconf, &bindaddr, prog, vers,
 				sendsz, recvsz);
 	if (cl) {