Welcome! Log In Create A New Profile

Advanced

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

Posted by Nikhil Kapoor 
Hi Everyone,

There is no limit for the value of inter parameter.
Same query was asked in the community :- https://discourse.haproxy.org/t/maximum-value-for-inter-parameter/2142

Here, I have created a patch (Version - 1.8.1) for limiting the maximum value of "inter" parameter.

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. 86400000 ms).
If user enter any value > 24h then parsing error would be displayed.

Please let me know if the patch is fine and can be merged to the main branch.

Thanks
Nikhil Kapoor
Problem:- No parsing error is displayed when larger value is given to "inter" parameter in config file.

A parsing error should be displayed on the screen.

Detailed steps:

1.Create 3 nodes setup(node1, node3 as backend servers, HAProxy is installed on node2.
2.Set 'inter 10000000000'in haproxy.config
3.Start Haproxy (systemctl restart haproxy)and check status of haproxy (#systemctl status haproxy)

When value of 'inter' parameter is "10000000000" in haproxy.cfg and haproxy service is started, no parsing error is displayed.

Solution:-
A check has been extended in parse_server() of server.c file, to verify the parameter value and display the parsing error if a value is greater than 24h(86400000 ms) process termination.


< /* The value of 'inter' parameter cannot be greater than 24 hours */
< if (val <= 0 || val >86400000) {
---
> if (val <= 0 ) {
Attachments:
open | download - inter_max_check.patch (498 bytes)
On Wed, 7 Mar 2018 at 09:50, Nikhil Kapoor <[email protected]> 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.
> 86400000 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?

J
--
Jonathan Matthews
London, UK
http://www.jpluscplusm.com/contact.html
Hi Jonathan,

On Wed, Mar 07, 2018 at 09:38:00PM +0000, Jonathan Matthews wrote:
> On Wed, 7 Mar 2018 at 09:50, Nikhil Kapoor <[email protected]> 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.
> > 86400000 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
Hi Willy,

-----Original Message-----
From: Willy Tarreau [mailto:[email protected]]
Sent: Thursday, March 8, 2018 10:18 PM
To: Jonathan Matthews <[email protected]>
Cc: Nikhil Kapoor <[email protected]>; haproxy@formilux.org
Subject: Re: BUG/MINOR: limiting the value of "inter" parameter for Health check

Hi Jonathan,

On Wed, Mar 07, 2018 at 09:38:00PM +0000, Jonathan Matthews wrote:
> On Wed, 7 Mar 2018 at 09:50, Nikhil Kapoor <[email protected]>
> 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.
> > 86400000 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.


After analyzing the code on the given problem, I found that the check is based on INT_MAX value and will only be executed when "stick-table" option is included in haproxy.cfg file.

However for a user, if "stick-table" option is not used in haproxy.cfg file then there is no other limit(24.85 days) in the haproxy code.
I believe the below check should not be based on "stick-table" option and should run every time when haproxy is started.
Please let me know if there is any gap in understanding and we can perform the code change as mentioned.

File-> cfgparse.c
--> if (val > INT_MAX ) {
ha_alert("parsing [%s:%d] : Expire value [%u]ms exceeds maxmimum value of 24.85 days.\n",
file, linenum, val);
err_code |= ERR_ALERT | ERR_FATAL;
goto out;

Regards
Nikhil Kapoor
Hi Willy,

>-----Original Message-----
>From: Nikhil Kapoor
>Sent: Tuesday, March 20, 2018 5:09 PM
>To: 'Willy Tarreau' <[email protected]>; Jonathan Matthews <[email protected]>
>Cc: haproxy@formilux.org
>Subject: RE: BUG/MINOR: limiting the value of "inter" parameter for Health check

>Hi Willy,

>-----Original Message-----
>From: Willy Tarreau [mailto:[email protected]]
>Sent: Thursday, March 8, 2018 10:18 PM
>To: Jonathan Matthews <[email protected]>
>Cc: Nikhil Kapoor <[email protected]>; haproxy@formilux.org
>Subject: Re: BUG/MINOR: limiting the value of "inter" parameter for Health check

>Hi Jonathan,

>On Wed, Mar 07, 2018 at 09:38:00PM +0000, Jonathan Matthews wrote:
>> On Wed, 7 Mar 2018 at 09:50, Nikhil Kapoor <[email protected]>
>> 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.
>> > 86400000 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.


>After analyzing the code on the given problem, I found that the check is based on INT_MAX value and will only be executed when >"stick-table" option is included in haproxy.cfg file.

>However for a user, if "stick-table" option is not used in haproxy.cfg file then there is no other limit(24.85 days) in the haproxy code.
>I believe the below check should not be based on "stick-table" option and should run every time when haproxy is started.
>Please let me know if there is any gap in understanding and we can perform the code change as mentioned.

>File-> cfgparse.c
>--> if (val > INT_MAX ) {
ha_alert("parsing [%s:%d] : Expire value [%u]ms exceeds maxmimum value of 24.85 days.\n",
file, linenum, val);
err_code |= ERR_ALERT | ERR_FATAL;
goto out;


Along with the above understanding I am starting with the patch creation. Please let me know if there is any gap.

Regards
Nikhil Kapoor
Sorry, only registered users may post in this forum.

Click here to login