[PATCH] MINOR: lua: allow changing port with set_addr

[Joseph C. Sible]

Add an optional port parameter, which can be either a number or a
string (to support '+' and '-' for port mapping).

This fixes issue #586.
---
 doc/lua-api/index.rst | 2 +-
 src/hlua_fcn.c| 7 ++-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/doc/lua-api/index.rst b/doc/lua-api/index.rst
index 9a578ee29..f3e5cb786 100644
--- a/doc/lua-api/index.rst
+++ b/doc/lua-api/index.rst
@@ -976,7 +976,7 @@ Server class
 server.
   :returns: an integer.
 
-.. js:function:: Server.set_addr(sv, addr)
+.. js:function:: Server.set_addr(sv, addr[, port])
 
   Dynamically change the address of the server. See the management socket
   documentation for more information about the format of the string.
diff --git a/src/hlua_fcn.c b/src/hlua_fcn.c
index a9c7fe507..e6f4d7379 100644
--- a/src/hlua_fcn.c
+++ b/src/hlua_fcn.c
@@ -1034,13 +1034,18 @@ int hlua_server_set_addr(lua_State *L)
 {
struct server *srv;
const char *addr;
+   const char *port;
const char *err;
 
srv = hlua_check_server(L, 1);
addr = luaL_checkstring(L, 2);
+   if (lua_gettop(L) >= 3)
+   port = luaL_checkstring(L, 3);
+   else
+   port = NULL;
 
HA_SPIN_LOCK(SERVER_LOCK, >lock);
-   err = server_parse_addr_change_request(srv, addr, "Lua script");
+   err = update_server_addr_port(srv, addr, port, "Lua script");
HA_SPIN_UNLOCK(SERVER_LOCK, >lock);
if (!err)
lua_pushnil(L);
-- 
2.25.1

Re: Question about connection settings proto fcgi check maxconn 9 minconn 5 maxqueue 0

[Aleksandar Lazic]

Hi Christopher.

On 04.05.20 11:28, Christopher Faulet wrote:
> Le 03/05/2020 à 09:52, Aleksandar Lazic a écrit :
>> Hi.
>>
>> I play a little bit with proto fcgi and see something what I don't 
>> understand.
>>
>> Hopefully someone can explain it a bit.
>>
>> My php-fpm have the following settings.
>>
>> ```
>> pm = dynamic
>> pm.max_children = 10
>> pm.min_spare_servers = 4
>> pm.start_servers = 5
>> pm.max_spare_servers = 6
>> pm.max_requests = 500
>> ```
>>
>> The haproxy server line have the following settings.
>> Btw: is fullconn deprecated or removed in 2.2 because it's still in the doc?
>> I don't know if the health check connection is counted in the maxconn value 
>> or not, is it?
>>
>> ```
>> server server1 127.0.0.1:9000 proto fcgi check maxconn 9 minconn 5 maxqueue 0
>> ```
>>
>> Now my assumption is that I should never get this message in the php-fpm 
>> output because maxconn is 9 and pm.max_children = 10 but I got it.
>>
>>
>> ```
>> [03-May-2020 07:27:44] WARNING: [pool www] server reached pm.max_children 
>> setting (10), consider raising it
>> ```
>>
>> This means that the connections to php-fpm are higher then maxcon configured 
>> in haproxy and this is what I not understand.
>> Here  some haproxy logs from the overload.
>>
> 
> 
> Hi Aleks,
> 
> I've made some tests on my side and the fork politic of php-fpm seems to be a 
> bit strange 
> because with the same config and the check disabled, I've the same warning 
> and 10 php-fpm 
> children with only 3 clients. When I checked the opened connections, I only 
> have 4 connections. 
> So there is no reason to have 10 children.

Thank you for your time answer.

>> ```
>> # normal run
>> 127.0.0.1:36466 [03/May/2020:07:37:55.995] myproxy phpservers/server1 
>> 0/69/0/2/71 200 66318 - -  50/50/8/4/0 0/9 "GET /pinf.php HTTP/1.1"
>> 127.0.0.1:36462 [03/May/2020:07:37:56.000] myproxy phpservers/server1 
>> 0/66/0/2/68 200 66318 - -  50/50/7/4/0 0/8 "GET /pinf.php HTTP/1.1"
>> 127.0.0.1:36488 [03/May/2020:07:37:56.002] myproxy phpservers/server1 
>> 0/66/0/2/109 200 66318 - -  50/50/6/4/0 0/8 "GET /pinf.php HTTP/1.1"
>> 127.0.0.1:36470 [03/May/2020:07:37:56.004] myproxy phpservers/server1 
>> 0/106/0/2/109 200 66318 - -  50/50/5/4/0 0/8 "GET /pinf.php HTTP/1.1"
>> 127.0.0.1:36546 [03/May/2020:07:37:56.007] myproxy phpservers/server1 
>> 0/106/0/2/108 200 66318 - -  50/50/4/4/0 0/8 "GET /pinf.php HTTP/1.1"
>>
>> # I think this a queued request
>> 127.0.0.1:36576 [03/May/2020:07:37:55.334] myproxy phpservers/server1 
>> 0/69/0/1402/1471 200 66318 - -  50/50/3/3/0 0/38 "GET /pinf.php HTTP/1.1"
>> 127.0.0.1:36484 [03/May/2020:07:37:55.363] myproxy phpservers/server1 
>> 0/41/0/1402/1443 200 66318 - -  50/50/3/3/0 0/39 "GET /pinf.php HTTP/1.1"
>>
> 
> Here, these both requests were not queued more longer than others (time spent 
> in queue is the second value, 69 and 41). But php-fpm was a quite long to 
> response. 1.4 seconds to have the response headers.
> 
>> 127.0.0.1:36480 [03/May/2020:07:37:55.363] myproxy phpservers/server1 
>> 0/41/0/2402/2444 200 66318 - -  50/50/1/1/0 0/40 "GET /pinf.php HTTP/1.1"
>> 127.0.0.1:36486 [03/May/2020:07:37:55.363] myproxy phpservers/server1 
>> 0/42/0/2402/2444 200 66318 - -  50/50/0/0/0 0/41 "GET /pinf.php HTTP/1.1"
> 
> And here, 2.4 seconds. It may be the reason why the health checks timed out.

This could be because php-fpm forks childs.

> So maybe there is too many opened connections. It may be a problem with idle 
> connections. 
> But you should not trust php-fpm warnings :) Monitor connections really 
> established instead.

Thanks I will go that way.

Re: [PATCH] CLEANUP: connections: align function declaration

[Olivier Houchard]

Hi William,

On Mon, May 04, 2020 at 02:08:54PM +0200, William Dauchy wrote:
> On Mon, May 04, 2020 at 01:56:10PM +0200, William Dauchy wrote:
> > Sorry for this one, I originally did update the server.h file, but I
> > don't know why I forgot to add it in my patch submission :/
> > then I saw you fixed it while pushing my commit, thank you for this.
> 
> I'm talking too fast, the .h was correctly sent.
> in fact it was a indeed a mistake added in between while pushing the
> fix. I lost myself between the different version here and there.
> Anyway :)

You did no wrong, this is actually my fault, I decided to make the
function static, as there were no need to export the symbol, so I
removed the changes from server.h, and added a static prototype at the
beginning of server.c, but quite obviously I forgot to add "static" to
the function definition :)
Your patch is now applied, thanks !

Olivier

Feature for ModSecurity: Real IP

[Jaime Brunicardi Esteban]

Hi,

Just to let you know I have added a feature to the modsecurity contrib to 
enable it to get the client Real IP Address through an extra param in the SPOE 
template. This way, standalone modsecurity has access to the REAL IP and you 
can enable features relying on the client real IP (DDOS, RBL, etc...)

https://github.com/jbrunicardi/haproxy-2.1/tree/master/contrib/modsecurity

[modsecurity]

   spoe-agent modsecurity-agent
  messages check-request
  option var-prefix modsec
  timeout hello  100ms
  timeout idle   30s
  timeout processing 15ms
  use-backend spoe-modsecurity

   spoe-message check-request
  args src unique-id method path query req.ver req.hdrs_bin req.body_size 
req.body
  event on-frontend-http-request


Maybe you find it interesting!

Best regards.

[itssimple]

Meytel S.L.
C/ Villamiel de Cáceres 13
28042 Madrid

¡Suscribete a nuestro newsletter!
https://www.meytel.net/suscribete-a-nuestro-newsletter/

*  91 741 65 79 / F: 91 320 78 24
directo: 91 142 05 55
https://www.meytel.net
@meytel
@nocmeytel

Status e intervenciones programadas
Portal de Soporte

horario oficina: 9-17 (l-j) 8-15 (v)
soporte: ¡abiertos 25 horas al día!

Re: [PATCH] CLEANUP: connections: align function declaration

[William Dauchy]

On Mon, May 04, 2020 at 01:56:10PM +0200, William Dauchy wrote:
> Sorry for this one, I originally did update the server.h file, but I
> don't know why I forgot to add it in my patch submission :/
> then I saw you fixed it while pushing my commit, thank you for this.

I'm talking too fast, the .h was correctly sent.
in fact it was a indeed a mistake added in between while pushing the
fix. I lost myself between the different version here and there.
Anyway :)

-- 
William

Re: [PATCH] CLEANUP: connections: align function declaration

[William Dauchy]

Hello Olivier,

Sorry for this one, I originally did update the server.h file, but I
don't know why I forgot to add it in my patch submission :/
then I saw you fixed it while pushing my commit, thank you for this.

On Mon, May 04, 2020 at 01:52:40PM +0200, William Dauchy wrote:
> this patch should be backported where commit 6318d33ce625
> ("BUG/MEDIUM: connections: force connections cleanup on server changes")
> will be backported, that is to say v1.9 to v2.1.
> 
> Fixes: 6318d33ce625 ("BUG/MEDIUM: connections: force connections cleanup
> on server changes")
> Signed-off-by: William Dauchy 
> ---
>  src/server.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/server.c b/src/server.c
> index 94e7aeed1..9e74fa485 100644
> --- a/src/server.c
> +++ b/src/server.c
> @@ -5169,7 +5169,7 @@ struct task *srv_cleanup_toremove_connections(struct 
> task *task, void *context,
>  /* cleanup connections for a given server
>   * might be useful when going on forced maintenance or live changing ip/port
>   */
> -void srv_cleanup_connections(struct server *srv)
> +static void srv_cleanup_connections(struct server *srv)
>  {
>   struct connection *conn;
>   int did_remove;
> -- 
> 2.26.2
> 

-- 
William

[PATCH] CLEANUP: connections: align function declaration

[William Dauchy]

this patch should be backported where commit 6318d33ce625
("BUG/MEDIUM: connections: force connections cleanup on server changes")
will be backported, that is to say v1.9 to v2.1.

Fixes: 6318d33ce625 ("BUG/MEDIUM: connections: force connections cleanup
on server changes")
Signed-off-by: William Dauchy 
---
 src/server.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/server.c b/src/server.c
index 94e7aeed1..9e74fa485 100644
--- a/src/server.c
+++ b/src/server.c
@@ -5169,7 +5169,7 @@ struct task *srv_cleanup_toremove_connections(struct task 
*task, void *context,
 /* cleanup connections for a given server
  * might be useful when going on forced maintenance or live changing ip/port
  */
-void srv_cleanup_connections(struct server *srv)
+static void srv_cleanup_connections(struct server *srv)
 {
struct connection *conn;
int did_remove;
-- 
2.26.2

Re: Question about connection settings proto fcgi check maxconn 9 minconn 5 maxqueue 0

[Christopher Faulet]

Le 03/05/2020 à 09:52, Aleksandar Lazic a écrit :

Hi.

I play a little bit with proto fcgi and see something what I don't understand.

Hopefully someone can explain it a bit.

My php-fpm have the following settings.

```
pm = dynamic
pm.max_children = 10
pm.min_spare_servers = 4
pm.start_servers = 5
pm.max_spare_servers = 6
pm.max_requests = 500
```

The haproxy server line have the following settings.
Btw: is fullconn deprecated or removed in 2.2 because it's still in the doc?
I don't know if the health check connection is counted in the maxconn value or 
not, is it?

```
server server1 127.0.0.1:9000 proto fcgi check maxconn 9 minconn 5 maxqueue 0
```

Now my assumption is that I should never get this message in the php-fpm output 
because maxconn is 9 and pm.max_children = 10 but I got it.


```
[03-May-2020 07:27:44] WARNING: [pool www] server reached pm.max_children 
setting (10), consider raising it
```

This means that the connections to php-fpm are higher then maxcon configured in 
haproxy and this is what I not understand.
Here  some haproxy logs from the overload.




Hi Aleks,

I've made some tests on my side and the fork politic of php-fpm seems to be a 
bit strange because with the same config and the check disabled, I've the same 
warning and 10 php-fpm children with only 3 clients. When I checked the opened 
connections, I only have 4 connections. So there is no reason to have 10 children.



```
# normal run
127.0.0.1:36466 [03/May/2020:07:37:55.995] myproxy phpservers/server1 0/69/0/2/71 200 
66318 - -  50/50/8/4/0 0/9 "GET /pinf.php HTTP/1.1"
127.0.0.1:36462 [03/May/2020:07:37:56.000] myproxy phpservers/server1 0/66/0/2/68 200 
66318 - -  50/50/7/4/0 0/8 "GET /pinf.php HTTP/1.1"
127.0.0.1:36488 [03/May/2020:07:37:56.002] myproxy phpservers/server1 0/66/0/2/109 200 
66318 - -  50/50/6/4/0 0/8 "GET /pinf.php HTTP/1.1"
127.0.0.1:36470 [03/May/2020:07:37:56.004] myproxy phpservers/server1 0/106/0/2/109 200 
66318 - -  50/50/5/4/0 0/8 "GET /pinf.php HTTP/1.1"
127.0.0.1:36546 [03/May/2020:07:37:56.007] myproxy phpservers/server1 0/106/0/2/108 200 
66318 - -  50/50/4/4/0 0/8 "GET /pinf.php HTTP/1.1"

# I think this a queued request
127.0.0.1:36576 [03/May/2020:07:37:55.334] myproxy phpservers/server1 0/69/0/1402/1471 
200 66318 - -  50/50/3/3/0 0/38 "GET /pinf.php HTTP/1.1"
127.0.0.1:36484 [03/May/2020:07:37:55.363] myproxy phpservers/server1 0/41/0/1402/1443 
200 66318 - -  50/50/3/3/0 0/39 "GET /pinf.php HTTP/1.1"



Here, these both requests were not queued more longer than others (time spent in 
queue is the second value, 69 and 41). But php-fpm was a quite long to response. 
1.4 seconds to have the response headers.



127.0.0.1:36480 [03/May/2020:07:37:55.363] myproxy phpservers/server1 0/41/0/2402/2444 
200 66318 - -  50/50/1/1/0 0/40 "GET /pinf.php HTTP/1.1"
127.0.0.1:36486 [03/May/2020:07:37:55.363] myproxy phpservers/server1 0/42/0/2402/2444 
200 66318 - -  50/50/0/0/0 0/41 "GET /pinf.php HTTP/1.1"


And here, 2.4 seconds. It may be the reason why the health checks timed out.

So maybe there is too many opened connections. It may be a problem with idle 
connections. But you should not trust php-fpm warnings :) Monitor connections 
really established instead.


--
Christopher Faulet

Re: [PATCH] guard tests that require pcre

[Илья Шипицин]

пн, 4 мая 2020 г. в 13:06, Christopher Faulet :

> Le 04/05/2020 à 08:57, Christopher Faulet a écrit :
> > Le 04/05/2020 à 07:46, William Lallemand a écrit :
> >> On Fri, May 01, 2020 at 12:57:06PM +0500, Илья Шипицин wrote:
> >>>
> >>> The following tests require pcre support:
> >>>
> >>> reg-tests/checks/4be_1srv_smtpchk_httpchk_layer47errors.vtc
> >>> reg-tests/checks/tls_health_checks.vtc
> >>
> >> Hello Ilya,
> >>
> >> I'm surprised because I don't see anything related to PCRE in these
> >> configurations. If it's related to the refactoring of the checks, it
> >> seems weird to me because these 2 VTCs doesn't have complex checks or
> >> tcp-checks.
> >>
> >
> > You're right William. There is nothing related to PCRE. A regex is used
> to
> > perform SMTP checks but it also works with POSIX regex. In fact, both
> tests fail
> > because of a bug in the H1 function h1_headers_to_hdr_list().
> >
> > It's strange I didn't noticed it till now because it makes HAProxy
> sefgault. I
> > will fix it soon.
>
> I pushed a fix. It should be ok now.
>

thank you, I will test.



>
> Thanks,
> --
> Christopher Faulet
>

Re: [RFC] Changing server port via Lua

[Christopher Faulet]

Le 03/05/2020 à 20:33, Joseph C. Sible a écrit :

A question was recently asked on Stack Overflow about how to change a
server's port via Lua [1]. The Lua function Server.set_addr currently
says "See the management socket documentation for more information
about the format of the string." However, this is misleading: the
management socket's "set server  addr" supports changing the port
with syntax like "192.0.2.1 port ", but that doesn't work in the
Lua function. It seems like it would be simple enough to change Lua to
use update_server_addr_port instead of update_server_addr, which would
let this work. I can think of two reasonable options for the Lua
interface:

1. Add a new optional argument to the function, so calls would look
like "server:set_addr("192.0.2.1", )
2. Parse the existing argument like the management socket does, so
calls would look like "server:set_addr("192.0.2.1 port ")

Is this something that would be accepted if I wrote a patch for it? If
so, which option should I go with for the Lua interface?

[1]: https://stackoverflow.com/q/61568148/7509065



Hi,

It seems to be reasonable. And, the first option is definitely better. Your 
patch will be welcome.


Thanks,
--
Christopher Faulet

Re: [PATCH] guard tests that require pcre

[Christopher Faulet]

Le 04/05/2020 à 08:57, Christopher Faulet a écrit :

Le 04/05/2020 à 07:46, William Lallemand a écrit :

On Fri, May 01, 2020 at 12:57:06PM +0500, Илья Шипицин wrote:


The following tests require pcre support:

reg-tests/checks/4be_1srv_smtpchk_httpchk_layer47errors.vtc
reg-tests/checks/tls_health_checks.vtc


Hello Ilya,

I'm surprised because I don't see anything related to PCRE in these
configurations. If it's related to the refactoring of the checks, it
seems weird to me because these 2 VTCs doesn't have complex checks or
tcp-checks.



You're right William. There is nothing related to PCRE. A regex is used to
perform SMTP checks but it also works with POSIX regex. In fact, both tests fail
because of a bug in the H1 function h1_headers_to_hdr_list().

It's strange I didn't noticed it till now because it makes HAProxy sefgault. I
will fix it soon.


I pushed a fix. It should be ok now.

Thanks,
--
Christopher Faulet

Re: [PATCH] guard tests that require pcre

[Christopher Faulet]

Le 04/05/2020 à 07:46, William Lallemand a écrit :

On Fri, May 01, 2020 at 12:57:06PM +0500, Илья Шипицин wrote:


The following tests require pcre support:

reg-tests/checks/4be_1srv_smtpchk_httpchk_layer47errors.vtc
reg-tests/checks/tls_health_checks.vtc


Hello Ilya,

I'm surprised because I don't see anything related to PCRE in these
configurations. If it's related to the refactoring of the checks, it
seems weird to me because these 2 VTCs doesn't have complex checks or
tcp-checks.



You're right William. There is nothing related to PCRE. A regex is used to 
perform SMTP checks but it also works with POSIX regex. In fact, both tests fail 
because of a bug in the H1 function h1_headers_to_hdr_list().


It's strange I didn't noticed it till now because it makes HAProxy sefgault. I 
will fix it soon.


--
Christopher Faulet