Re: lua socket api settimeout in seconds vs. milliseconds

[Tim Düsterhus]

Thierry,

Am 08.03.2018 um 21:15 schrieb Thierry Fournier:
> Hey, the example of use of socket.http in attachment of your original
> commit is great !

If you are curious what I built with that and in case you missed my list
mail advertising the project:
https://github.com/TimWolla/haproxy-auth-request

> 3 new patch in attachement to consider for the initial subject of
> this thread.

I took a look at it:

> The LuaSocket documentation tell anything about the returned value,
> but the effective code set an integer of value one.

Should this read: "The LuaSocket documentation __does not__ tell
anything about the returned value ..."?

There's a few more regular typos (spelling of words) in the commit
messages, but that specific part is misleadingly worded, thus I wanted
to make you aware of it.

Best regards
Tim Düsterhus

Re: Need some help in HAPROXY setup.

[Aleksandar Lazic]

Hi Amit.

Please keep the mailing list in loop, thanks.

Am 08.03.2018 um 17:03 schrieb amit raj:
> Hello Alex,
> 
> Two things we have to achieve with the HAPROXY.
> 
> 1.There should be no RSTs for closing the connection.(which is happening
> when we are keeping option http-keep-alive in ur frontend and backend.)

Have you tried this options?

https://cbonte.github.io/haproxy-dconv/1.8/configuration.html#4-timeout%20server-fin

https://cbonte.github.io/haproxy-dconv/1.8/configuration.html#4-timeout%20client-fin

Why is the RST a problem?

> 2.Even load balancing not only with session load balancing also with the
> bytes in and bytes out , as you can see on the HAPROXy stats page.
> (We have used option-http tunnel for reducing the  RST ,but due to this
> our even load balancing is screwed up for bytes in and bytes out).

Let me describe what I have understood.

You want to use more the one balancing decision base.

* session (cookie, url, ...)
* bytes in. I assume from the client
* bytes out. I assume from the client

I this right?

Please can you describe in a ASCII flow how the decision flow should work.

What I have seen in the doc are this acl samples

https://cbonte.github.io/haproxy-dconv/1.8/configuration.html#7.3.1-table_bytes_in_rate
https://cbonte.github.io/haproxy-dconv/1.8/configuration.html#7.3.1-table_bytes_out_rate

But I'm not sure if this is what you need.

> Following our HA proxy config.
> 
> frontend test_ha
>     bind.  :
>     option http-tunnel
>     default_backend test_back
> #--
> # backend instances
> #--
> backend test_back
>     balance roundrobin
>     option no-http-tunnel
>     option http-keep-alive
>     option http-reuse aggresssive

That's a short conf and not a working one.
Please post the current conf which works, thanks.

> Please help us .

Best regards
Aleks

Re: lua socket api settimeout in seconds vs. milliseconds

[Thierry Fournier]

> On 8 Mar 2018, at 15:14, Tim Düsterhus  wrote:
> 
> Hi
> 
> Am 08.03.2018 um 15:10 schrieb Thierry Fournier:
>> Ok, Lua expect the number of elements ins the stack. The right way for 
>> returning 1 is:
>> 
>>   lua_pushinteger(L, 1);
>>return 1;
>> 
> 
> Okay, then my patch probably worked, because of whatever value was left
> on the stack. I learned something new here!
> 
> Indeed the original socket class pushes `1` to the stack (but it uses
> lua_pushnumber, instead of pushinteger). For the reference here's the
> original line of code:
> https://github.com/diegonehab/luasocket/blob/316a9455b9cb4637fe6e62b20fbe05f5141fec54/src/timeout.c#L172


Thanks for the search int the LuaSocket code. The doc doesn’t tell
anything about the returned value.

I think that the original patch worked because the top of the stack
was the timeout value given as argument.

Hey, the example of use of socket.http in attachment of your original
commit is great !

3 new patch in attachement to consider for the initial subject of
this thread.

BR,
Thierry



0001-BUG-MINOR-lua-the-function-returns-anything.patch
Description: Binary data


0002-BUG-MINOR-lua-funtion-hlua_socket_settimeout-don-t-c.patch
Description: Binary data


0003-MINOR-lua-typo-fix.patch
Description: Binary data

Re: cppcheck finding

[Olivier Houchard]

Hi,

On Thu, Mar 08, 2018 at 05:44:31PM +0100, Willy Tarreau wrote:
> Hi,
> 
> On Wed, Mar 07, 2018 at 03:26:25PM +0500,  ??? wrote:
> > Hello,
> > 
> > [src/proto_uxst.c:160]: (warning) Redundant assignment of
> > 'xfer_sock->next->prev' to itself.
> > 
> > is it in purpose ?
> 
> I suspect it's a mistake and that it was meant to be xfer_sock->prev instead.
> CCing Olivier to double-check.
> 

Oops, you're right, good catch !
The attached patch should fix it.

Regards,

Olivier
>From 32b505d6093bad96eb4a65272bd3e7b3aad4767b Mon Sep 17 00:00:00 2001
From: Olivier Houchard 
Date: Thu, 8 Mar 2018 18:25:49 +0100
Subject: [PATCH] MINOR: unix: Don't mess up when removing the socket from the
 xfer_sock_list.

When removing the socket from the xfer_sock_list, we want to set
next->prev to prev, not to next->prev, which is useless.

This should be backported to 1.8.
---
 src/proto_uxst.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/proto_uxst.c b/src/proto_uxst.c
index 3ab637f20..0f717385e 100644
--- a/src/proto_uxst.c
+++ b/src/proto_uxst.c
@@ -157,7 +157,7 @@ static int uxst_find_compatible_fd(struct listener *l)
if (xfer_sock->prev)
xfer_sock->prev->next = xfer_sock->next;
if (xfer_sock->next)
-   xfer_sock->next->prev = xfer_sock->next->prev;
+   xfer_sock->next->prev = xfer_sock->prev;
free(xfer_sock);
}
return ret;
-- 
2.14.3

Re: BUG/MINOR: limiting the value of "inter" parameter for Health check

[Willy Tarreau]

Hi Jonathan,

On Wed, Mar 07, 2018 at 09:38:00PM +, Jonathan Matthews wrote:
> On Wed, 7 Mar 2018 at 09:50, Nikhil Kapoor  wrote
> 
> > As currently, no parsing error is displayed when larger value is given to
> > "inter" parameter in config file.
> >
> > After applying this patch the maximum value of "inter" is set to 24h (i.e.
> > 8640 ms).
> >
> 
> I regret to inform you, with no little embarrassment, that some years ago I
> designed a system which relied upon this parameter being set higher than 24
> hours.
> 
> I was not proud of this system, and it served absolutely minimal quantities
> of traffic ... but it was a valid setup.
> 
> What's the rationale for having *any* maximum value here - saving folks
> from unintentional misconfigurations, or something else?

I agree with you here. In fact what we should do is instead strengthen
the timeout parser to emit errors when the parsed number overflows. The
timer wraps around 49.7 days with a sliding window of half of it allowing
24.85 usable days to always be possible. That would be preferable and it
wouldn't set any arbitrary had limits.

Cheers,
Willy

Re: cppcheck finding

[Willy Tarreau]

Hi,

On Wed, Mar 07, 2018 at 03:26:25PM +0500,  ??? wrote:
> Hello,
> 
> [src/proto_uxst.c:160]: (warning) Redundant assignment of
> 'xfer_sock->next->prev' to itself.
> 
> is it in purpose ?

I suspect it's a mistake and that it was meant to be xfer_sock->prev instead.
CCing Olivier to double-check.

Thanks!
Willy

Re: lua socket api settimeout in seconds vs. milliseconds

[Tim Düsterhus]

Hi

Am 08.03.2018 um 15:10 schrieb Thierry Fournier:
> Ok, Lua expect the number of elements ins the stack. The right way for 
> returning 1 is:
> 
>lua_pushinteger(L, 1);
> return 1;
> 

Okay, then my patch probably worked, because of whatever value was left
on the stack. I learned something new here!

Indeed the original socket class pushes `1` to the stack (but it uses
lua_pushnumber, instead of pushinteger). For the reference here's the
original line of code:
https://github.com/diegonehab/luasocket/blob/316a9455b9cb4637fe6e62b20fbe05f5141fec54/src/timeout.c#L172

Best regards
Tim Düsterhus

Re: lua socket api settimeout in seconds vs. milliseconds

[Thierry Fournier]

> On 8 Mar 2018, at 15:03, Tim Düsterhus  wrote:
> 
> Thierry,
> 
> Am 08.03.2018 um 10:24 schrieb Thierry Fournier:
>> I forgot 3 things while my first read:
>> 
>> - The Lua error are not trigerred with a return 1 (the return 1 is a bug
>>   in the original function), but with something like that:
>> 
> 
> Your first patch will be regressing my commit
> 119a5f10e47f3507e58116002583e1226473485d, which specifically changed the
> return value to be 1 for compatibility with the standard Socket class of
> Lua!
> 
> http://git.haproxy.org/?p=haproxy.git;a=commit;h=119a5f10e47f3507e58116002583e1226473485d


Ok, Lua expect the number of elements ins the stack. The right way for 
returning 1 is:

   lua_pushinteger(L, 1);
return 1;

I will add this in the patch.

BR,
Thierry

Re: lua socket api settimeout in seconds vs. milliseconds

[Tim Düsterhus]

Thierry,

Am 08.03.2018 um 10:24 schrieb Thierry Fournier:
> I forgot 3 things while my first read:
> 
>  - The Lua error are not trigerred with a return 1 (the return 1 is a bug
>in the original function), but with something like that:
> 

Your first patch will be regressing my commit
119a5f10e47f3507e58116002583e1226473485d, which specifically changed the
return value to be 1 for compatibility with the standard Socket class of
Lua!

http://git.haproxy.org/?p=haproxy.git;a=commit;h=119a5f10e47f3507e58116002583e1226473485d

Best regards
Tim Düsterhus

Re: lua socket api settimeout in seconds vs. milliseconds

[Thierry Fournier]

Hi Mark,

Thanks, it seems perfect. But, I can’t apply on current master branch, the
patch is rejected.

I forgot 3 things while my first read:

 - The Lua error are not trigerred with a return 1 (the return 1 is a bug
   in the original function), but with something like that:

   WILL_LJMP(luaL_error(L, "settimeout: cannot set negatives values"));

   The return became useless because the luaL_error() function never returns.
   (it soes a longjmp call).

 - I think that timeouts < 1s are allowed. The cli refuse these ones,
   but the HAProxy core is ready to work with timeout less than 1s. Note
   that, When you remove this check, negative timeouts could be accepted
   (the check < 1000 chek also for negative values) and this is a bug.

 - The doc (doc/lua-api/index.rst) should be updated (actuelly, line 1899:
   .. js:function:: Socket.settimeout(socket, value [, mode])). It will be
   necessary to precise that the fucntion accept Number in place of Integer
   and, number with fractional part are allowed.

I join two of my fixes. The third patch is yours, check if you agree with
the content and the commit message.

BR,
Thierry




0001-BUG-MINOR-lua-the-function-returns-anything.patch
Description: Binary data


0002-BUG-MINOR-lua-funtion-hlua_socket_settimeout-don-t-c.patch
Description: Binary data


0003-MINOR-lua-typo-fix.patch
Description: Binary data




> On 8 Mar 2018, at 01:17, Mark Lakes  wrote:
> 
> Hi Thierry, thanks for feedback. Addressed concerns in the new attached patch.
> 
> http://w3.impa.br/~diego/software/luasocket/tcp.html#settimeout
> 
> Description: instead of hlua_socket_settimeout() accepting only integers, 
> allow user to specify float and
> double as well. Convert to milliseconds much like cli_parse_set_timeout but 
> also sanity check the value.
> 
> -mark
> 
> 
> On Wed, Mar 7, 2018 at 9:55 AM, Thierry Fournier  
> wrote:
> Hi Mark,
> 
> Thanks for the patch. I don’t like usage of floating point, but the
> luasocket documentation says that the settimeout() function accept only
> second. In this case, the usage of floating point seems be to be a good
> way.
> 
> Can you split in a second commit the fix of comments from the effective
> patch, and avoid this kind of changes:
> 
>-int tmout;
>+inttmout;
> 
> Just because, this kind of changes are useless, and it add noisy
> information in the patch.
> 
> A last point: could you explain int the message of the patch the
> goal of these patch. To avoid a search, this is the link of the official
> luasocket setimeout function:
> 
> http://w3.impa.br/~diego/software/luasocket/tcp.html#settimeout
> 
> Thanks
> Thierry
> 
> 
> > On 7 Mar 2018, at 18:16, Mark Lakes  wrote:
> >
> > In regards to earlier conversation, herein is a patch attached for the 
> > feature.
> > From the mail archive:
> > https://www.mail-archive.com/haproxy@formilux.org/msg27806.html
> > https://www.mail-archive.com/haproxy@formilux.org/msg27807.html
> >
> > Mark Lakes
> > Signal Sciences | www.signalsciences.com |
> >
> > conversation participants:
> > Willy Tarreau
> > Adis Nezirovic
> > Nick Galbreath
> >
> > - Last conversation and decision agreement --
> > Nick Galbreath Thu, 09 Nov 2017 20:44:28 -0800
> >
> > thanks wily.
> >
> > re: " CONTRIBUTING in the sources directory," -
> >
> > yes, that is what I was looking for!  thanks for the tip.
> >
> > re:  least it seems important to round up non-null values to the next
> > millisecond.
> >
> > Definitely, we can and should add some checks for invalid values, etc.
> >
> > I'll read CONTRIBUTING, and set up my dev env, try a patch,  and report
> > back appropriately.
> >
> > regards,
> >
> > n
> >
> > On Thu, Nov 9, 2017 at 8:37 PM, Willy Tarreau  > > wrote:
> >
> > > Hi Nick,
> > >
> > > On Thu, Nov 09, 2017 at 08:27:29PM -0800, Nick Galbreath wrote:
> > > > Hello Adis,
> > > >
> > > > We could certainly add another API/Lua function but it might be easier 
> > > > to
> > > > change
> > > >
> > > > luaL_checkinteger(L, 2) in
> > > >
> > > >  tmout = MAY_LJMP(luaL_checkinteger(L, 2)) * 1000;
> > > >
> > > > to  luaL_checknumber(L, 2), along with appropriate cast to int.
> > > >
> > > > Then we have backwards compatibility, less documentation to write, and
> > > get
> > > > millisecond timeouts.
> > >
> > > At least it seems important to round up non-null values to the next
> > > millisecond, otherwise we may observe busy loops when users specify
> > > sleep delays smaller than the millisecond, as haproxy's internal
> > > clock is millisecond-based (poll()'s resolution).
> > >
> >
> > > > If people want a separate API, I'm happy to do that too, just more work.
> > >
> > > I think it should work as you propose it, more or less the round up of
> > > course.
> > >
> > > > Please advise, and I'll make a patch either way.  I'm unfamiliar with 
> > > > the
> > > > HAProxy development 

Re: Feature suggestion: Check for same binding on multiple frontends

[Lukas Tribus]

Hello,


On 8 March 2018 at 06:36, Moomjian, Chad  wrote:
> Thanks for the information, Lukas. I'm confused why this is not a default 
> option though. Can you think of a time when you would ever want the exact 
> same binding in multiple places in the config?

noreuseport is not something that reads the configuration and applies
some kind of heuristics for such errors, what it does is that it makes
haproxy NOT set the SO_REUSEPORT [1] socket option.

SO_REUSEPORT is useful because it permits faster and (depending on
more factors) hitless reloads. Other than that an important use-case
is to have the kernel load-balance between different sockets on
different CPU cores for scalability reasons.



Lukas


[1] https://lwn.net/Articles/542629/