Re: [PR] Add srvkey option to stick-table
Thanks for your feedback. I have some followup questions.
> I think you should initialized this ebtree with EB_ROOT_UNIQUE as
value.
I'm not sure I understand what the distinction between EB_ROOT and
EB_ROOT_UNIQUE is. I'm happy to add this, but I'd like to understand
why it should be EB_ROOT_UNIQUE here (since it looks like it is
initialized as EB_ROOT for used_server_id and used_listener_id, and I
can't find where it is initialized for used_server_name, which I
believe would be equivalent since EB_ROOT is basically just zero
initialized).
> I think this is dangerous to initialize two objects with the same value
as a string. I would strdup() to initialize s->addr_node.key.
ok. I was following the pattern used with conf.name in struct server.
Would it be better to use strdup, or to not have a separate field for
addr_desc, and just use addr_node.key (which is a void*)? If the
latter maybe rename addr_node as addr_desc?
Additionally:
In srv_set_addr_desc, would it be better to hold the lock over the
entire operation, or just while we are actually manipulating the tree?
Meaning should I release the lock between deleting the old node and
inserting the new node? And should this use the existing proxy lock,
or should there be a separate dedicated lock for used_server_addr?
I'm also a little bit concerned about leaking memory in the
server_key_dict. I couldn't find anything that cleans up these keys,
although there is a refcount on them. In an environment that uses
service discovery and the servers change frequently, this could lead
to a memory leak as this dict only grows in size.
Thank you,
Thayne
Thayne McCombs
Senior Software Engineer
tha...@lucidchart.com
Lucid.co
On Tue, Dec 1, 2020 at 10:24 AM Frederic Lecaille wrote:
>
> On 12/1/20 11:50 AM, Emeric Brun wrote:
> > Hi,
>
> Hi Thayne,
>
> See my answers below.
>
> > On 11/30/20 10:23 AM, PR Bot wrote:
> >> Dear list!
> >>
> >> Author: Thayne McCombs
> >> Number of patches: 2
> >>
> >> This is an automated relay of the Github pull request:
> >> Add srvkey option to stick-table
> >>
> >> Patch title(s):
> >> Add srvkey option to stick-table
> >> Harden sa2str agains 107-byte-long abstract unix domain path
> >>
> >> Link:
> >> https://github.com/haproxy/haproxy/pull/979
> >>
> >> Edit locally:
> >> wget https://github.com/haproxy/haproxy/pull/979.patch && vi 979.patch
> >>
> >> Apply locally:
> >> curl https://github.com/haproxy/haproxy/pull/979.patch | git am -
> >>
> >> Description:
> >> This allows using the address of the server rather than the name of
> >> the
> >> server for keeping track of servers in a backend for
> >> stickiness.
> >>
> >> Fixes #814
> >>
> >> I haven't tested this at all
> >> yet, and it still needs some polish, but here is a draft of how to fix
> >> #814.
> >>
> >> This is my first significant contribution to haproxy,
> >> so I would not be surprised if I'm doing something terribly wrong, and
> >> I'm sure there are at least some small mistakes in it. Initial
> >> feedback would be very welcome.
>
> Thank you for this first contribution which looks almost good!!! We all
> do wrong things ;) Even if it not easy to comment your code because it
> is not included in this mail, let's giving a try.
>
>
> I have noticed two places where initializations are missing:
>
>
> diff --git a/include/haproxy/proxy-t.h b/include/haproxy/proxy-t.h
> index 998e210f6..e62b79765 100644
> --- a/include/haproxy/proxy-t.h
> +++ b/include/haproxy/proxy-t.h
> @@ -424,6 +424,7 @@ struct proxy {
> char *lfsd_file;/* file name where the
> structured-data logformat
> string for RFC5424 appears (strdup) */
> int lfsd_line; /* file name where the
> structured-data logformat
> string for RFC5424 appears */
> } conf; /* config information */
> + struct eb_root used_server_addr;/* list of server addresses in
> use */
>
>
> < used_server_addr> ebtree is not initialized. This would make haproxy
> crash. I think you should initialized this ebtree with EB_ROOT_UNIQUE as
> value.
>
>
> You should comment all the new functions (see srv_set_addr_desc() and
> perhaps others).
>
> The following function should be commente as this done for
> srv_set_dyncookie() espcially for the locks we need even if this
> function is only used at parsing time, as far as I see. What if we want
> to use it at runing time? This function should be called with a lock on
> server (not necessary at parsing time).
>
>
> +static void srv_set_addr_desc(struct server *s)
> +{
> + struct proxy *p = s->proxy;
> + char *key;
> +
> + key = sa2str(>addr, s->svc_port, s->flags & SRV_F_MAPPORTS);
> +
> + if (strcmp(key, s->addr_desc) == 0) {
>
> ->addr_desc is initialized only by srv_set_addr_desc(). So the first
> time we enter this function ->addr_desc has NULL as value.
>
>
stable-bot: Bugfixes waiting for a release 2.1 (12), 2.0 (11)
Hi, This is a friendly bot that watches fixes pending for the next haproxy-stable release! One such e-mail is sent periodically once patches are waiting in the last maintenance branch, and an ideal release date is computed based on the severity of these fixes and their merge date. Responses to this mail must be sent to the mailing list. Last release 2.1.10 was issued on 2020-11-05. There are currently 12 patches in the queue cut down this way: - 2 MAJOR, first one merged on 2020-11-13 - 2 MEDIUM, first one merged on 2020-11-13 - 8 MINOR, first one merged on 2020-11-06 Thus the computed ideal release date for 2.1.11 would be 2020-12-11, which is in two weeks or less. Last release 2.0.19 was issued on 2020-11-06. There are currently 11 patches in the queue cut down this way: - 2 MAJOR, first one merged on 2020-11-13 - 2 MEDIUM, first one merged on 2020-11-13 - 7 MINOR, first one merged on 2020-11-13 Thus the computed ideal release date for 2.0.20 would be 2020-12-11, which is in two weeks or less. The current list of patches in the queue is: - 2.0, 2.1 - MAJOR : spoe: Be sure to remove all references on a released spoe applet - 2.0, 2.1 - MAJOR : filters: Always keep all offsets up to date during data filtering - 2.0, 2.1 - MEDIUM : peers: fix decoding of multi-byte length in stick-table messages - 2.0, 2.1 - MEDIUM : filters: Forward all filtered data at the end of http filtering - 2.0, 2.1 - MINOR : http-fetch: Extract cookie value even when no cookie name - 2.0, 2.1 - MINOR : lua: set buffer size during map lookups - 2.1 - MINOR : http-htx: Just warn if payload of an errorfile doesn't match the C-L - 2.0, 2.1 - MINOR : http-fetch: Fix calls w/o parentheses of the cookie sample fetches - 2.0, 2.1 - MINOR : peers: Do not ignore a protocol error for dictionary entries. - 2.0, 2.1 - MINOR : pattern: a sample marked as const could be written - 2.0, 2.1 - MINOR : peers: Missing TX cache entries reset. - 2.0, 2.1 - MINOR : http-ana: Don't wait for the body of CONNECT requests -- The haproxy stable-bot is freely provided by HAProxy Technologies to help improve the quality of each HAProxy release. If you have any issue with these emails or if you want to suggest some improvements, please post them on the list so that the solutions suiting the most users can be found.
Re: [PR] Add srvkey option to stick-table
On 12/1/20 11:50 AM, Emeric Brun wrote:
Hi,
Hi Thayne,
See my answers below.
On 11/30/20 10:23 AM, PR Bot wrote:
Dear list!
Author: Thayne McCombs
Number of patches: 2
This is an automated relay of the Github pull request:
Add srvkey option to stick-table
Patch title(s):
Add srvkey option to stick-table
Harden sa2str agains 107-byte-long abstract unix domain path
Link:
https://github.com/haproxy/haproxy/pull/979
Edit locally:
wget https://github.com/haproxy/haproxy/pull/979.patch && vi 979.patch
Apply locally:
curl https://github.com/haproxy/haproxy/pull/979.patch | git am -
Description:
This allows using the address of the server rather than the name of
the
server for keeping track of servers in a backend for
stickiness.
Fixes #814
I haven't tested this at all
yet, and it still needs some polish, but here is a draft of how to fix
#814.
This is my first significant contribution to haproxy,
so I would not be surprised if I'm doing something terribly wrong, and
I'm sure there are at least some small mistakes in it. Initial
feedback would be very welcome.
Thank you for this first contribution which looks almost good!!! We all
do wrong things ;) Even if it not easy to comment your code because it
is not included in this mail, let's giving a try.
I have noticed two places where initializations are missing:
diff --git a/include/haproxy/proxy-t.h b/include/haproxy/proxy-t.h
index 998e210f6..e62b79765 100644
--- a/include/haproxy/proxy-t.h
+++ b/include/haproxy/proxy-t.h
@@ -424,6 +424,7 @@ struct proxy {
char *lfsd_file; /* file name where the structured-data logformat
string for RFC5424 appears (strdup) */
int lfsd_line; /* file name where the structured-data logformat
string for RFC5424 appears */
} conf; /* config information */
+ struct eb_root used_server_addr;/* list of server addresses in
use */
< used_server_addr> ebtree is not initialized. This would make haproxy
crash. I think you should initialized this ebtree with EB_ROOT_UNIQUE as
value.
You should comment all the new functions (see srv_set_addr_desc() and
perhaps others).
The following function should be commente as this done for
srv_set_dyncookie() espcially for the locks we need even if this
function is only used at parsing time, as far as I see. What if we want
to use it at runing time? This function should be called with a lock on
server (not necessary at parsing time).
+static void srv_set_addr_desc(struct server *s)
+{
+ struct proxy *p = s->proxy;
+ char *key;
+
+ key = sa2str(>addr, s->svc_port, s->flags & SRV_F_MAPPORTS);
+
+ if (strcmp(key, s->addr_desc) == 0) {
->addr_desc is initialized only by srv_set_addr_desc(). So the first
time we enter this function ->addr_desc has NULL as value.
+ free(key);
+ return;
+ }
+
+ HA_RWLOCK_WRLOCK(PROXY_LOCK, >lock);
+ ebpt_delete(>addr_node);
+ HA_RWLOCK_WRUNLOCK(PROXY_LOCK, >lock);
+
+ free(s->addr_desc);
+ s->addr_desc = key;
+ s->addr_node.key = key;
I think this is dangerous to initialize two objects with the same value
as a string. I would strdup() to initialize s->addr_node.key.
+ // TODO: should this use a dedicated lock?
+ HA_RWLOCK_WRLOCK(PROXY_LOCK, >lock);
+ ebis_insert(>used_server_addr, >addr_node);
+ HA_RWLOCK_WRUNLOCK(PROXY_LOCK, >lock);
+}
+
There are two typos in the documentation:
+ a template). If "addr" is given, then the server is idntified
^
+ by it's current network address
^
You should split your patch to provide such cleanup code in seperated patch:
@@ -1453,7 +1497,7 @@ int url2sa(const char *url, int ulen, struct
sockaddr_storage *addr, struct spli
/* Secondly, if :// pattern is found, verify parsed stuff
* before pattern is matching our http pattern.
* If so parse ip address and port in uri.
-*
+*
I am not sure at all about this modification, but it semms
implementation dependent to me:
diff --git a/src/tools.c b/src/tools.c
index c51f542c6..21e3f06ef 100644
--- a/src/tools.c
+++ b/src/tools.c
@@ -1228,7 +1228,7 @@ char * sa2str(const struct sockaddr_storage *addr,
int port, int map_ports)
case AF_UNIX:
path = ((struct sockaddr_un *)addr)->sun_path;
if (path[0] == '\0') {
- return memprintf(, "abns@%s", path+1);
+ return memprintf(, "abns@%107s", path+1);
} else {
return strdup(path);
}
I would not implement this function:
+static void srv_addr_updated(struct server *srv)
+{
+ srv_set_dyncookie(srv);
+ srv_set_addr_desc(srv);
+}
Note something important: there are
Fwd: Forthcoming OpenSSL Release
FYI -- Forwarded message - From: Paul Nelson Date: Tue, 1 Dec 2020 at 11:15 Subject: Forthcoming OpenSSL Release To: The OpenSSL project team would like to announce the forthcoming release of OpenSSL version 1.1.1i. This release will be made available on Tuesday 8th December 2020 between 1300-1700 UTC. OpenSSL 1.1.i is a security-fix release. The highest severity issue fixed in this release is HIGH: https://www.openssl.org/policies/secpolicy.html#high Yours The OpenSSL Project Team
Re: [PR] Add srvkey option to stick-table
Hi, On 11/30/20 10:23 AM, PR Bot wrote: > Dear list! > > Author: Thayne McCombs > Number of patches: 2 > > This is an automated relay of the Github pull request: >Add srvkey option to stick-table > > Patch title(s): >Add srvkey option to stick-table >Harden sa2str agains 107-byte-long abstract unix domain path > > Link: >https://github.com/haproxy/haproxy/pull/979 > > Edit locally: >wget https://github.com/haproxy/haproxy/pull/979.patch && vi 979.patch > > Apply locally: >curl https://github.com/haproxy/haproxy/pull/979.patch | git am - > > Description: >This allows using the address of the server rather than the name of >the >server for keeping track of servers in a backend for >stickiness. > >Fixes #814 > >I haven't tested this at all >yet, and it still needs some polish, but here is a draft of how to fix >#814. > >This is my first significant contribution to haproxy, >so I would not be surprised if I'm doing something terribly wrong, and >I'm sure there are at least some small mistakes in it. Initial >feedback would be very welcome. > > Instructions: >This github pull request will be closed automatically; patch should be >reviewed on the haproxy mailing list (haproxy@formilux.org). Everyone is >invited to comment, even the patch's author. Please keep the author and >list CCed in replies. Please note that in absence of any response this >pull request will be lost. > I'm CCing Fred This pull request seems the extension for stick on server adresses we discussed a long time Ago when Fred implements the stick on server name. Willy exposed what to do here (using the dictionnary etc..) https://github.com/haproxy/haproxy/issues/814 I think Fred is more accurate on the subject and implemntation to perform the pull request revue. R, Emeric
Bid Writing, Major Donors and Volunteering Workshops
NFP WORKSHOPS 18 Blake Street, York YO1 8QG 01133 280988 Affordable Training Courses for Charities, Schools & Public Sector Organisations This email has been sent to haproxy@formilux.org CLICK TO UNSUBSCRIBE FROM LIST Alternatively send a blank e-mail to unsubscr...@nfpmail2001.co.uk quoting haproxy@formilux.org in the subject line. Unsubscribe requests will take effect within seven days. Bid Writing: The Basics Online via ZOOM COST £95 TOPICS COVERED Do you know the most common reasons for rejection? Are you gathering the right evidence? Are you making the right arguments? Are you using the right terminology? Are your numbers right? Are you learning from rejections? Are you assembling the right documents? Do you know how to create a clear and concise standard funding bid? Are you communicating with people or just excluding them? Do you know your own organisation well enough? Are you thinking through your projects carefully enough? Do you know enough about your competitors? Are you answering the questions funders will ask themselves about your application? Are you submitting applications correctly? PARTICIPANTS Staff members, volunteers, trustees or board members of charities, schools, not for profits or public sector organisations who intend to submit grant funding applications to charitable grant making trusts and foundations. People who provide advice to these organisations are also welcome. Bid Writing: Advanced Online via ZOOM COST £95 TOPICS COVERED Are you applying to the right trusts? Are you applying to enough trusts? Are you asking for the right amount of money? Are you applying in the right ways? Are your projects the most fundable projects? Are you carrying out trust fundraising in a professional way? Are you delegating enough work? Are you highly productive or just very busy? Are you looking for trusts in all the right places? How do you compare with your competitors for funding? Is the rest of your fundraising hampering your bids to trusts? Do you understand what trusts are ideally looking for? PARTICIPANTS Staff members, volunteers, trustees or board members of charities, schools, not for profits or public sector organisations who intend to submit grant funding applications to charitable grant making trusts and foundations. People who provide advice to these organisations are also welcome. Dates & Booking Links BID WRITING: THE BASICS Mon 07 Dec 2020 10.00 to 12.30Booking Link Mon 21 Dec 2020 10.00 to 12.30Booking Link Mon 11 Jan 2020 10.00 to 12.30Booking Link Mon 25 Jan 2020 10.00 to 12.30Booking Link Mon 08 Feb 2020 10.00 to 12.30Booking Link Mon 22 Feb 2020 10.00 to 12.30Booking Link BID WRITING: ADVANCED Tue 08 Dec 2020 10.00 to 12.30Booking Link Tue 22 Dec 2020 10.00 to 12.30Booking Link Tue 12 Jan 2020 10.00 to 12.30Booking Link Tue 26 Jan 2020 10.00 to 12.30Booking Link Tue 09 Feb 2020 10.00 to 12.30Booking Link Tue 23 Feb 2020 10.00 to 12.30Booking Link Recruiting and Managing Volunteers Online via ZOOM COST £195 TOPICS COVERED Where do you find volunteers? How do you find the right volunteers? How do you attract volunteers? How do you run volunteer recruitment events? How do you interview volunteers? How do you train volunteers? How do you motivate volunteers? How do you involve volunteers? How do you recognise volunteer? How do you recognise problems with volunteers? How do you learn from volunteer problems? How do you retain volunteers? How do you manage volunteers? What about volunteers and your own staff? What about younger, older and employee volunteers? PARTICIPANTS Staff members, volunteers, trustees or board members of charities, schools, not for profits or public sector organisations who intend to recruit volunteers into their organisation and then manage those volunteers. People who provide advice to these organisations are also welcome. Dates & Booking Links RECRUITING AND MANAGING VOLUNTEERS Wed 09 Dec 2020 10.00 to 16.00Booking Link Wed 13 Jan 2021 10.00 to 16.00Booking Link Wed 10 Mar 2021 10.00 to 16.00Booking Link Major Donor Fundraising Online via ZOOM COST £95 TOPICS COVERED Major Donor Characteristics, Motivations and Requirements. Researching and Screening Major Donors. Encouraging, Involving and Retaining Major Donors. Building Relationships with Major Donors. Major Donor Events and Activities. Setting Up Major Donor Clubs.Asking For Major Gifts. Looking After and Reporting Back to Major Donors. Delivering on Major Donor Expectations. Showing Your Appreciation to Major Donors. Fundraising Budgets and Committees. PARTICIPANTS Staff members, volunteers, trustees or board members of charities, schools, not for profits or public sector organisations who intend to carry out Major Donor Fundraising. People who provide advice to these organisations are also welcome. Dates & Booking Links MAJOR DONOR FUNDRAISING Thu 10 Dec 2020 10.00 to 12.30Booking Link Wed 10
Re: [PR] Skip unsupported ciphers for ecdsa cert
Thanks, they are now enabled. I've fixed boringssl builds and tested it with libressl locally as well. I will work within my fork until I'm happy with the code before I submit it again. I made the code more efficient but want to do a bit more testing and need to clean up the commit messages. https://github.com/markun/haproxy/pull/1 On Tue, Dec 1, 2020 at 10:05 AM Илья Шипицин wrote: > You can enable github actions on your fork (by default actions are > disabled) > > It should start several builds including libressl and boringssl > > On Tue, Dec 1, 2020, 1:14 AM Marcoen Hirschberg wrote: > >> Good point, I just tried with boringssl and compilation failed. Thanks >> for pointing that out. >> >> On Mon, Nov 30, 2020 at 8:28 PM Илья Шипицин >> wrote: >> >>> will it run on LibreSSL, BoringSSL ? >>> >>> вт, 1 дек. 2020 г. в 00:26, PR Bot : >>> Dear list! Author: Marcoen Hirschberg Number of patches: 3 This is an automated relay of the Github pull request: Skip unsupported ciphers for ecdsa cert Patch title(s): MINOR: ssl: variable renames for clarity MINOR: ssl: skip unknown client cipher BUG/MINOR: ssl: only choose ECDSA cert if server and client have common ECDSA ciphers Link: https://github.com/haproxy/haproxy/pull/983 Edit locally: wget https://github.com/haproxy/haproxy/pull/983.patch && vi 983.patch Apply locally: curl https://github.com/haproxy/haproxy/pull/983.patch | git am - Description: Instructions: This github pull request will be closed automatically; patch should be reviewed on the haproxy mailing list (haproxy@formilux.org). Everyone is invited to comment, even the patch's author. Please keep the author and list CCed in replies. Please note that in absence of any response this pull request will be lost.
Re: [PR] Skip unsupported ciphers for ecdsa cert
You can enable github actions on your fork (by default actions are disabled) It should start several builds including libressl and boringssl On Tue, Dec 1, 2020, 1:14 AM Marcoen Hirschberg wrote: > Good point, I just tried with boringssl and compilation failed. Thanks for > pointing that out. > > On Mon, Nov 30, 2020 at 8:28 PM Илья Шипицин wrote: > >> will it run on LibreSSL, BoringSSL ? >> >> вт, 1 дек. 2020 г. в 00:26, PR Bot : >> >>> Dear list! >>> >>> Author: Marcoen Hirschberg >>> Number of patches: 3 >>> >>> This is an automated relay of the Github pull request: >>>Skip unsupported ciphers for ecdsa cert >>> >>> Patch title(s): >>>MINOR: ssl: variable renames for clarity >>>MINOR: ssl: skip unknown client cipher >>>BUG/MINOR: ssl: only choose ECDSA cert if server and client have >>> common ECDSA ciphers >>> >>> Link: >>>https://github.com/haproxy/haproxy/pull/983 >>> >>> Edit locally: >>>wget https://github.com/haproxy/haproxy/pull/983.patch && vi >>> 983.patch >>> >>> Apply locally: >>>curl https://github.com/haproxy/haproxy/pull/983.patch | git am - >>> >>> Description: >>> >>> >>> Instructions: >>>This github pull request will be closed automatically; patch should be >>>reviewed on the haproxy mailing list (haproxy@formilux.org). >>> Everyone is >>>invited to comment, even the patch's author. Please keep the author >>> and >>>list CCed in replies. Please note that in absence of any response this >>>pull request will be lost. >>> >>>
Re: [PATCH] DOC: Clarify %HP description in log-format
On Tue, Dec 01, 2020 at 08:53:51AM +0100, Maciej Zdeb wrote: > I was scratching my head for a long time with this. "HTTP path only" sounds > much better to me. I've adjusted the patch. OK, looks good, now merged, thank you! Willy
