[PATCH v2] remove automatic port id generation
Dan Williams
dcbw at redhat.com
Wed Jun 17 07:19:19 PDT 2015
On Wed, 2015-06-17 at 15:49 +0200, Julien Courtat wrote:
> Since kernel can automatically set a unique portid to each socket any
> application would bind with, there is no use to keep the portid generation
> procedure made of 22 bits for the pid and 10 bits left for a pseudo
> random value.
I haven't looked, but how far back does that kernel functionality go?
Has it always been there? The port code has been there since the
initial 2007 import to git, so I just feel like there must have been
some reason Thomas Graf wrote it...
Dan
> Remove this procedure and let kernel choose a port id for the socket.
> It is still possible for an application to set the port id manually via
> nl_socket_set_local_port().
>
> Signed-off-by: Julien Courtat <julien.courtat at 6wind.com>
> ---
> lib/nl.c | 47 +++---------------
> lib/socket.c | 153 ----------------------------------------------------------
> 2 files changed, 7 insertions(+), 193 deletions(-)
>
> diff --git a/lib/nl.c b/lib/nl.c
> index 8fc9ec1..d0828fc 100644
> --- a/lib/nl.c
> +++ b/lib/nl.c
> @@ -126,47 +126,14 @@ int nl_connect(struct nl_sock *sk, int protocol)
> if (err < 0)
> goto errout;
>
> - if (_nl_socket_is_local_port_unspecified (sk)) {
> - uint32_t port;
> - uint32_t used_ports[32] = { 0 };
> -
> - while (1) {
> - port = _nl_socket_generate_local_port_no_release(sk);
> -
> - if (port == UINT32_MAX) {
> - NL_DBG(4, "nl_connect(%p): no more unused local ports.\n", sk);
> - _nl_socket_used_ports_release_all(used_ports);
> - err = -NLE_EXIST;
> - goto errout;
> - }
> - err = bind(sk->s_fd, (struct sockaddr*) &sk->s_local,
> - sizeof(sk->s_local));
> - if (err == 0)
> - break;
> -
> - errsv = errno;
> - if (errsv == EADDRINUSE) {
> - NL_DBG(4, "nl_connect(%p): local port %u already in use. Retry.\n", sk, (unsigned) port);
> - _nl_socket_used_ports_set(used_ports, port);
> - } else {
> - NL_DBG(4, "nl_connect(%p): bind() for port %u failed with %d (%s)\n",
> - sk, (unsigned) port, errsv, strerror_r(errsv, buf, sizeof(buf)));
> - _nl_socket_used_ports_release_all(used_ports);
> - err = -nl_syserr2nlerr(errsv);
> - goto errout;
> - }
> - }
> - _nl_socket_used_ports_release_all(used_ports);
> - } else {
> - err = bind(sk->s_fd, (struct sockaddr*) &sk->s_local,
> - sizeof(sk->s_local));
> - if (err != 0) {
> - errsv = errno;
> - NL_DBG(4, "nl_connect(%p): bind() failed with %d (%s)\n",
> + err = bind(sk->s_fd, (struct sockaddr*) &sk->s_local,
> + sizeof(sk->s_local));
> + if (err != 0) {
> + errsv = errno;
> + NL_DBG(4, "nl_connect(%p): bind() failed with %d (%s)\n",
> sk, errsv, strerror_r(errsv, buf, sizeof(buf)));
> - err = -nl_syserr2nlerr(errsv);
> - goto errout;
> - }
> + err = -nl_syserr2nlerr(errsv);
> + goto errout;
> }
>
> addrlen = sizeof(local);
> diff --git a/lib/socket.c b/lib/socket.c
> index b29d1da..8aa909e 100644
> --- a/lib/socket.c
> +++ b/lib/socket.c
> @@ -59,120 +59,6 @@ static void __init init_default_cb(void)
> }
> }
>
> -static uint32_t used_ports_map[32];
> -static NL_RW_LOCK(port_map_lock);
> -
> -static uint32_t generate_local_port(void)
> -{
> - int i, j, n, m;
> - static uint16_t idx_state = 0;
> - uint32_t pid = getpid() & 0x3FFFFF;
> -
> - nl_write_lock(&port_map_lock);
> -
> - if (idx_state == 0) {
> - uint32_t t = time(NULL);
> -
> - /* from time to time (on average each 2^15 calls), the idx_state will
> - * be zero again. No problem, just "seed" anew with time(). */
> - idx_state = t ^ (t >> 16) ^ 0x3047;
> - } else
> - idx_state = idx_state + 20011; /* add prime number */
> -
> - i = idx_state >> 5;
> - n = idx_state;
> - for (j = 0; j < 32; j++) {
> - /* walk the index somewhat randomized, with always leaving the block
> - * #0 as last. The reason is that libnl-1 will start at block #0,
> - * so just leave the first 32 ports preferably for libnl-1 owned sockets
> - * (this is relevant only if the applications ends up using both versions
> - * of the library and doesn't hurt otherwise). */
> - if (j == 31)
> - i = 0;
> - else
> - i = (((i-1) + 7) % 31) + 1;
> -
> - if (used_ports_map[i] == 0xFFFFFFFF)
> - continue;
> -
> - for (m = 0; m < 32; m++) {
> - n = (n + 13) % 32;
> - if (1UL & (used_ports_map[i] >> n))
> - continue;
> -
> - used_ports_map[i] |= (1UL << n);
> - n += (i * 32);
> -
> - /* PID_MAX_LIMIT is currently at 2^22, leaving 10 bit
> - * to, i.e. 1024 unique ports per application. */
> -
> - nl_write_unlock(&port_map_lock);
> -
> - return pid + (((uint32_t)n) << 22);
> - }
> - }
> -
> - nl_write_unlock(&port_map_lock);
> -
> - /* Out of sockets in our own PID namespace, what to do? FIXME */
> - NL_DBG(1, "Warning: Ran out of unique local port namespace\n");
> - return UINT32_MAX;
> -}
> -
> -static void release_local_port(uint32_t port)
> -{
> - int nr;
> - uint32_t mask;
> -
> - if (port == UINT32_MAX)
> - return;
> -
> - BUG_ON(port == 0);
> -
> - nr = port >> 22;
> - mask = 1UL << (nr % 32);
> - nr /= 32;
> -
> - nl_write_lock(&port_map_lock);
> - BUG_ON((used_ports_map[nr] & mask) != mask);
> - used_ports_map[nr] &= ~mask;
> - nl_write_unlock(&port_map_lock);
> -}
> -
> -/** \cond skip */
> -void _nl_socket_used_ports_release_all(const uint32_t *used_ports)
> -{
> - int i;
> -
> - for (i = 0; i < 32; i++) {
> - if (used_ports[i] != 0) {
> - nl_write_lock(&port_map_lock);
> - for (; i < 32; i++) {
> - BUG_ON((used_ports_map[i] & used_ports[i]) != used_ports[i]);
> - used_ports_map[i] &= ~(used_ports[i]);
> - }
> - nl_write_unlock(&port_map_lock);
> - return;
> - }
> - }
> -}
> -
> -void _nl_socket_used_ports_set(uint32_t *used_ports, uint32_t port)
> -{
> - int nr;
> - int32_t mask;
> -
> - nr = port >> 22;
> - mask = 1UL << (nr % 32);
> - nr /= 32;
> -
> - /*
> - BUG_ON(port == UINT32_MAX || port == 0 || (getpid() & 0x3FFFFF) != (port & 0x3FFFFF));
> - BUG_ON(used_ports[nr] & mask);
> - */
> -
> - used_ports[nr] |= mask;
> -}
> /** \endcond */
>
> /**
> @@ -251,9 +137,6 @@ void nl_socket_free(struct nl_sock *sk)
> if (sk->s_fd >= 0)
> close(sk->s_fd);
>
> - if (!(sk->s_flags & NL_OWN_PORT))
> - release_local_port(sk->s_local.nl_pid);
> -
> nl_cb_put(sk->s_cb);
> free(sk);
> }
> @@ -329,28 +212,6 @@ void nl_socket_enable_auto_ack(struct nl_sock *sk)
> sk->s_flags &= ~NL_NO_AUTO_ACK;
> }
>
> -/** @} */
> -
> -/** \cond skip */
> -int _nl_socket_is_local_port_unspecified(struct nl_sock *sk)
> -{
> - return (sk->s_local.nl_pid == 0);
> -}
> -
> -uint32_t _nl_socket_generate_local_port_no_release(struct nl_sock *sk)
> -{
> - uint32_t port;
> -
> - /* reset the port to generate_local_port(), but do not release
> - * the previously generated port. */
> -
> - port = generate_local_port();
> - sk->s_flags &= ~NL_OWN_PORT;
> - sk->s_local.nl_pid = port;
> - return port;
> -}
> -/** \endcond */
> -
> /**
> * @name Source Idenficiation
> * @{
> @@ -358,18 +219,6 @@ uint32_t _nl_socket_generate_local_port_no_release(struct nl_sock *sk)
>
> uint32_t nl_socket_get_local_port(const struct nl_sock *sk)
> {
> - if (sk->s_local.nl_pid == 0) {
> - /* modify the const argument sk. This is justified, because
> - * nobody ever saw the local_port from externally. So, we
> - * initilize it on first use.
> - *
> - * Note that this also means that you cannot call this function
> - * from multiple threads without synchronization. But nl_sock
> - * is not automatically threadsafe anyway, so the user is not
> - * allowed to do that.
> - */
> - return _nl_socket_generate_local_port_no_release((struct nl_sock *) sk);
> - }
> return sk->s_local.nl_pid;
> }
>
> @@ -387,8 +236,6 @@ uint32_t nl_socket_get_local_port(const struct nl_sock *sk)
> */
> void nl_socket_set_local_port(struct nl_sock *sk, uint32_t port)
> {
> - if (!(sk->s_flags & NL_OWN_PORT))
> - release_local_port(sk->s_local.nl_pid);
> sk->s_flags |= NL_OWN_PORT;
> sk->s_local.nl_pid = port;
> }
More information about the libnl
mailing list