UlricE / pen

Pen
Other
250 stars 41 forks source link

Hash initial server selection without save last server #18

Closed mvrogowski closed 8 years ago

mvrogowski commented 8 years ago

We use pen to load balancing RADIUS server that handles standard and PEAP requests.

The standard RADIUS request is a simple transaction that works fine with any load balancing algorithm. But the PEAP transaction is more complex: it needs the TLS tunnel to encrypt the traffic. To perform a PEAP transaction, it need to do more than one request to server. To get success, we need to ensure that all transaction packets will sent to same server. The only way to do it, is using the hash algorithm.

When hash algorithm is enabled, it redirects all request to same server, making one server hammered while others are on idle state.

To fix our problem, I added a new initial server selection algorithm called "Hash no server" that adds the source port to hash algorithm and don't save the last server the client used.

Could you analyze my solution? If it is a good one, I will be thankful if you add it to your repository. :)

I hope I have contributed to your project.

Thank's for your attention!

UlricE commented 8 years ago

Will have a look as soon as I get access to a computer.

mvrogowski commented 8 years ago

Could you add a new tar.gz release to http://siag.nu/pub/pen/ with this mods, please?

UlricE commented 8 years ago

Figuring out how to add this without breaking backward compatibility.

UlricE commented 8 years ago

About pull request #18:

One of the points of the hash algorithm is that it is deterministic:

  1. In a load-balanced pair of load balancers, the server selection is done the same way regardless of where the client ends up.
  2. A client will always be sent to the same server if it connects from the same IP address.

PR 18 maintains property 1 but not 2, so breaks the old hash algorithm.

The DSR code has its own hash algorithm which includes the port when the -r (roundrobin) option is used. It would be a good idea to use the same semantics for the non-DSR case.

Using -r will forego reusing the old server (see initial_server).

With these changes (see below), -hr will do the same thing as -N while maintaining backwards compatibility.

Have a look and if it makes sense I'll put this in 0.31.0.

diff --git a/client.c b/client.c
index cbdc327..79bd25a 100644
--- a/client.c
+++ b/client.c
@@ -84,10 +84,6 @@ int store_client(struct sockaddr_storage *cli)
        clients[i].addr = *cli;
        clients[i].connects++;

-       /* don't remember server */
-       if(server_alg & ALG_HASH_NO_SERVER) {
-               clients[i].server = NO_SERVER;
-       }

        DEBUG(2, "Client %s has index %d", pen_ntoa(cli), i);

diff --git a/pen.c b/pen.c
index 7f66433..eb346e7 100644
--- a/pen.c
+++ b/pen.c
@@ -2415,9 +2415,6 @@ static int options(int argc, char **argv)
                        }
                        break;
 #endif  /* HAVE_LIBSSL */
-               case 'N':
-                       server_alg |= ALG_HASH_NO_SERVER;
-                       break;
                case '?':
                default:
                        usage();
diff --git a/server.c b/server.c
index b9c016a..0238ef6 100644
--- a/server.c
+++ b/server.c
@@ -51,7 +51,11 @@ static int pen_hash(struct sockaddr_storage *a)
        switch (a->ss_family) {
        case AF_INET:
                si = (struct sockaddr_in *)a;
-               hash = (si->sin_addr.s_addr ^ si->sin_port) % nservers;
+               if (server_alg & ALG_ROUNDROBIN) {
+                       hash = (si->sin_addr.s_addr ^ si->sin_port) % nservers;
+               } else {
+                       hash = si->sin_addr.s_addr % nservers;
+               }

                DEBUG(2, "Hash: %d", hash);

@@ -206,7 +210,7 @@ int initial_server(int conn)
        }
        if (server_alg & ALG_PRIO) return server_by_prio();
        if (server_alg & ALG_WEIGHT) return server_by_weight();
-       if (server_alg & ALG_HASH || server_alg & ALG_HASH_NO_SERVER) return pen_hash(&clients[conns[conn].client].addr);
+       if (server_alg & ALG_HASH) return pen_hash(&clients[conns[conn].client].addr);
        return server_by_roundrobin();
 }

diff --git a/server.h b/server.h
index 53a67d8..b89d0aa 100644
--- a/server.h
+++ b/server.h
@@ -11,7 +11,6 @@
 #define ALG_PRIO 8
 #define ALG_HASH 16
 #define ALG_STUBBORN 32
-#define ALG_HASH_NO_SERVER 64

 #define EMERGENCY_SERVER (-1)
 #define ABUSE_SERVER (-2)
mvrogowski commented 8 years ago

I will run some tests and reply as soon it's done.

mvrogowski commented 8 years ago

I tested your solution and all works very well!

Please, tell me when you add to 0.31 and make it public.

Thank's, Ulric!

UlricE commented 8 years ago

Pushed and released. :+1: