2021-11-02 03:09:22

by NeilBrown

Subject: [PATCH nfs-utils] mount: don't bind a socket needlessly.

When clnt_ping() calls get_socket(), get_socket() will create a socket,
call bind() to choose an unused local port, and then connect to the
given address.

The "bind()" call is unnecessary and problematic.
It is unnecessary as the "connect()" call will bind the socket as
required.
It is problematic as it requires a completely unused port number, rather
than just a port number which isn't currently in use for connecting to
the given remote address.
If all local ports (net.ipv4.ip_local_port_range) are in use, the bind()
will fail. However the connect() call will only fail if all those port
are in use for connecting to the same address.

So remove the unnecessary bind() call.

Signed-off-by: NeilBrown <[email protected]>
---
utils/mount/network.c | 4 ----
1 file changed, 4 deletions(-)

diff --git a/utils/mount/network.c b/utils/mount/network.c
index e803dbbe5a2c..35261171a615 100644
--- a/utils/mount/network.c
+++ b/utils/mount/network.c
@@ -429,10 +429,6 @@ static int get_socket(struct sockaddr_in *saddr, unsigned int p_prot,
if (resvp) {
if (bindresvport(so, &laddr) < 0)
goto err_bindresvport;
- } else {
- cc = bind(so, SAFE_SOCKADDR(&laddr), namelen);
- if (cc < 0)
- goto err_bind;
}
if (type == SOCK_STREAM || (conn && type == SOCK_DGRAM)) {
cc = connect_to(so, SAFE_SOCKADDR(saddr), namelen,
--
2.33.1

2021-11-10 19:16:54

by Steve Dickson

Subject: Re: [PATCH nfs-utils] mount: don't bind a socket needlessly.

Hello,

On 11/1/21 23:08, NeilBrown wrote:
>
> When clnt_ping() calls get_socket(), get_socket() will create a socket,
> call bind() to choose an unused local port, and then connect to the
> given address.
>
> The "bind()" call is unnecessary and problematic.
> It is unnecessary as the "connect()" call will bind the socket as
> required.
> It is problematic as it requires a completely unused port number, rather
> than just a port number which isn't currently in use for connecting to
> the given remote address.
> If all local ports (net.ipv4.ip_local_port_range) are in use, the bind()
> will fail. However the connect() call will only fail if all those port
> are in use for connecting to the same address.
>
> So remove the unnecessary bind() call.
>
> Signed-off-by: NeilBrown <[email protected]>
Committed... (tag: nfs-utils-2-5-5-rc4)

steved.

> ---
> utils/mount/network.c | 4 ----
> 1 file changed, 4 deletions(-)
>
> diff --git a/utils/mount/network.c b/utils/mount/network.c
> index e803dbbe5a2c..35261171a615 100644
> --- a/utils/mount/network.c
> +++ b/utils/mount/network.c
> @@ -429,10 +429,6 @@ static int get_socket(struct sockaddr_in *saddr, unsigned int p_prot,
> if (resvp) {
> if (bindresvport(so, &laddr) < 0)
> goto err_bindresvport;
> - } else {
> - cc = bind(so, SAFE_SOCKADDR(&laddr), namelen);
> - if (cc < 0)
> - goto err_bind;
> }
> if (type == SOCK_STREAM || (conn && type == SOCK_DGRAM)) {
> cc = connect_to(so, SAFE_SOCKADDR(saddr), namelen,
>