Re: commit 493d9dc makes a SVN-checkout stall..
Hi Olivier, Willy, Just to confirm, as expected it (c3500c3) indeed works for me :). Thanks for the quick fix! Regards, PiBa-NL (Pieter) Op 25-3-2020 om 17:16 schreef Willy Tarreau: On Wed, Mar 25, 2020 at 05:08:03PM +0100, Olivier Houchard wrote: That is... interesting, not sure I reached such an outstanding result. Oh I stopped trying to guess long ago :-) This is now fixed, sorry about that ! Confirmed, much better now, thanks! Willy
Re: commit 493d9dc makes a SVN-checkout stall..
On Wed, Mar 25, 2020 at 05:08:03PM +0100, Olivier Houchard wrote: > That is... interesting, not sure I reached such an outstanding result. Oh I stopped trying to guess long ago :-) > This is now fixed, sorry about that ! Confirmed, much better now, thanks! Willy
Re: commit 493d9dc makes a SVN-checkout stall..
Hi Willy, On Wed, Mar 25, 2020 at 02:03:56PM +0100, Willy Tarreau wrote: > On Wed, Mar 25, 2020 at 12:40:33PM +0100, Olivier Houchard wrote: > > I think I figured it out, and commit > > 69664419d209d2f64dbcd90f991e7ec2efff2ee2 > > should fix it, at least now I can do a full svn checkout. > > > > Can you confirm it does the trick for you ? > > Olivier, I think you made a last minute change after your fix because > it broke the build: > > https://travis-ci.com/github/haproxy/haproxy/jobs/302517378 > > src/mux_h1.c:2495:23: error: incompatible pointer types passing 'struct > connection **' to parameter of type 'const struct buffer *' > [-Werror,-Wincompatible-pointer-types] > if (unlikely(b_data(>conn))) > ^~ > include/common/compiler.h:98:40: note: expanded from macro 'unlikely' > #define unlikely(x) (__builtin_expect((x) != 0, 0)) >^ > include/common/buf.h:100:50: note: passing argument to parameter 'b' here > static inline size_t b_data(const struct buffer *b) > ^ > I think it was h1c->dbuf not conn. > > Willy That is... interesting, not sure I reached such an outstanding result. This is now fixed, sorry about that ! Olivier
Re: commit 493d9dc makes a SVN-checkout stall..
On Wed, Mar 25, 2020 at 12:40:33PM +0100, Olivier Houchard wrote: > I think I figured it out, and commit 69664419d209d2f64dbcd90f991e7ec2efff2ee2 > should fix it, at least now I can do a full svn checkout. > > Can you confirm it does the trick for you ? Olivier, I think you made a last minute change after your fix because it broke the build: https://travis-ci.com/github/haproxy/haproxy/jobs/302517378 src/mux_h1.c:2495:23: error: incompatible pointer types passing 'struct connection **' to parameter of type 'const struct buffer *' [-Werror,-Wincompatible-pointer-types] if (unlikely(b_data(>conn))) ^~ include/common/compiler.h:98:40: note: expanded from macro 'unlikely' #define unlikely(x) (__builtin_expect((x) != 0, 0)) ^ include/common/buf.h:100:50: note: passing argument to parameter 'b' here static inline size_t b_data(const struct buffer *b) ^ I think it was h1c->dbuf not conn. Willy
Re: commit 493d9dc makes a SVN-checkout stall..
On Wed, Mar 25, 2020 at 04:06:27PM +0500, ??? wrote: > btw, is there some tool to test at least RFC 7230 compliance ? Not that I'm aware of, especially given the complexity of the HTTP/1 specs which try to cover all known use cases and implementations. > also, there are various specific implementation like MAPI / MS RPC which > might require some testing tool as well > > inspired by h2spec actually. can/should we add some HTTP/1.1 tests as well ? I don't think so. H2 is extremely simple and strict in its definition, this is not the case for H1 and complying is more a matter of how strict you are with the SHOULD or MAY than anything else. I'd say that in practice most of the VTC files already rely on some basic compliance and are already OK for this. We can always improve, of course, but this shouldn't deserve unreasonable efforts. Willy
Re: commit 493d9dc makes a SVN-checkout stall..
Hi Pieter, On Wed, Mar 25, 2020 at 01:42:14AM +0100, Olivier Houchard wrote: > Hi Pieter, > > On Wed, Mar 25, 2020 at 01:14:46AM +0100, PiBa-NL wrote: > > Hi List, Willy, > > > > Today i thought lets give v2.2-dev5 a try for my production environment ;). > > Soon it turned out to cause SVN-Checkout to stall/disconnect for a > > repository we run locally in a Collab-SVN server. > > > > I tracked it down to this commit: 493d9dc (MEDIUM: mux-h1: do not blindly > > wake up the tasklet at end of request anymore) causing the problem for the > > first time. Is there something tricky there that can be suspected to cause > > the issue.? Perhaps a patch i can try? > > > > While 'dissecting' the issue i deleted the whole directory each time and > > performed a new svn-checkout several times. It doesn't always stall at the > > exact same point but usually after checking out around +- 20 files something > > between 0.5 and 2 MB. , the commit before that one allows me to checkout > > 500+MB through haproxy without issue.. A wireshark seems to show that > > haproxy is sending several of RST,ACK packets for a 4 different connections > > to the svn-server at the same milisecond after it was quiet for 2 seconds.. > > The whole issue happens in a timeframe of start of checkout till when it > > stalls within 15 seconds. > > > > The 'nokqueue' i usually try on my FreeBSD machine doesn't change anything. > > > > Hope you have an idea where to look. Providing captures/logs is a bit > > difficult without some careful scrubbing.. > > > > Regards, > > PiBa-NL (Pieter) > > > > No answer yet, but I just tried to do a checkout of the FreeBSD svn tree > via haproxy, and it indeed doesn't work, it's a bit late right now, but > I'll have a look tomorrow :) > I think I figured it out, and commit 69664419d209d2f64dbcd90f991e7ec2efff2ee2 should fix it, at least now I can do a full svn checkout. Can you confirm it does the trick for you ? Thanks ! Olivier
Re: commit 493d9dc makes a SVN-checkout stall..
btw, is there some tool to test at least RFC 7230 compliance ? also, there are various specific implementation like MAPI / MS RPC which might require some testing tool as well inspired by h2spec actually. can/should we add some HTTP/1.1 tests as well ? ср, 25 мар. 2020 г. в 11:02, Willy Tarreau : > Hi guys, > > On Wed, Mar 25, 2020 at 01:42:14AM +0100, Olivier Houchard wrote: > (...) > > > Hope you have an idea where to look. Providing captures/logs is a bit > > > difficult without some careful scrubbing.. > > I think a good test would be to just revert that patch, or selectively > each one of the 3 lines it contains. > > > No answer yet, but I just tried to do a checkout of the FreeBSD svn tree > > via haproxy, and it indeed doesn't work, it's a bit late right now, but > > I'll have a look tomorrow :) > > OK thanks. I remember that you had some opinions on some of them at some > point. However I'm tempted to think that if any of them needs to be > changed, > we're probably missing something somewhere which could indicate that other > subscribes might randomly fail, because each time it's in h1_detach() and > I don't see why we should always wake up if already subscribed. > > Thanks, > Willy > >
[PATCHES] dns related
Hi there,
A couple of patches here to cleanup and fix some bugs introduced
by 13a9232ebc63fdf357ffcf4fa7a1a5e77a1eac2b.
Baptiste
From 801e4f1d7ad1f9858f4b646fc4badebab3b46715 Mon Sep 17 00:00:00 2001
From: Baptiste Assmann
Date: Wed, 19 Feb 2020 00:53:26 +0100
Subject: [PATCH 1/2] CLEANUP: remove obsolete comments
This patch removes some old comments introduced by
13a9232ebc63fdf357ffcf4fa7a1a5e77a1eac2b.
Those comments are related to issues already fixed.
---
src/dns.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/src/dns.c b/src/dns.c
index bbc4f4ac1..3e52e1731 100644
--- a/src/dns.c
+++ b/src/dns.c
@@ -1030,7 +1030,6 @@ static int dns_validate_dns_response(unsigned char *resp, unsigned char *bufend,
/* now parsing additional records */
nb_saved_records = 0;
- //TODO: check with Dinko for DNS poisoning
for (i = 0; i < dns_p->header.arcount; i++) {
if (reader >= bufend)
return DNS_RESP_INVALID;
@@ -1202,7 +1201,6 @@ static int dns_validate_dns_response(unsigned char *resp, unsigned char *bufend,
continue;
tmp_record->ar_item = dns_answer_record;
}
- //TODO: there is a leak for now, since we don't clean up AR records
LIST_ADDQ(_p->ar_list, _answer_record->list);
}
--
2.17.1
From 9c5f4f464380a1f67c7d5d802d6c05c0086cebfe Mon Sep 17 00:00:00 2001
From: Baptiste Assmann
Date: Wed, 19 Feb 2020 01:08:51 +0100
Subject: [PATCH 2/2] BUG/MEDIUM: dns: improper parsing of aditional records
13a9232ebc63fdf357ffcf4fa7a1a5e77a1eac2b introduced parsing of
Additionnal DNS response section to pick up IP address when available.
That said, this introduced a side effect for other query types (A and
) leading to consider those responses invalid when parsing the
Additional section.
This patch avoids this situation by ensuring the Additional section is
parsed only for SRV queries.
---
src/dns.c | 26 ++
1 file changed, 6 insertions(+), 20 deletions(-)
diff --git a/src/dns.c b/src/dns.c
index 3e52e1731..953f9414c 100644
--- a/src/dns.c
+++ b/src/dns.c
@@ -1028,7 +1028,9 @@ static int dns_validate_dns_response(unsigned char *resp, unsigned char *bufend,
/* Save the number of records we really own */
dns_p->header.ancount = nb_saved_records;
- /* now parsing additional records */
+ /* now parsing additional records for SRV queries only */
+ if (dns_query->type != DNS_RTYPE_SRV)
+ goto skip_parsing_additional_records;
nb_saved_records = 0;
for (i = 0; i < dns_p->header.arcount; i++) {
if (reader >= bufend)
@@ -1043,25 +1045,7 @@ static int dns_validate_dns_response(unsigned char *resp, unsigned char *bufend,
if (len == 0) {
pool_free(dns_answer_item_pool, dns_answer_record);
- return DNS_RESP_INVALID;
- }
-
- /* Check if the current record dname is valid. previous_dname
- * points either to queried dname or last CNAME target */
- if (dns_query->type != DNS_RTYPE_SRV && memcmp(previous_dname, tmpname, len) != 0) {
- pool_free(dns_answer_item_pool, dns_answer_record);
- if (i == 0) {
-/* First record, means a mismatch issue between
- * queried dname and dname found in the first
- * record */
-return DNS_RESP_INVALID;
- }
- else {
-/* If not the first record, this means we have a
- * CNAME resolution error */
-return DNS_RESP_CNAME_ERROR;
- }
-
+ continue;
}
memcpy(dns_answer_record->name, tmpname, len);
@@ -1206,6 +1190,8 @@ static int dns_validate_dns_response(unsigned char *resp, unsigned char *bufend,
}
} /* for i 0 to arcount */
+ skip_parsing_additional_records:
+
/* Save the number of records we really own */
dns_p->header.arcount = nb_saved_records;
--
2.17.1
Re: [PATCH] MINOR: ssl: skip self issued CA in cert chain for ssl_ctx
Hi, Patch rebase from master. > Le 6 mars 2020 à 17:06, Emmanuel Hocdet a écrit : > > Hi, > > > Patch proposal. > I will update the documentation if this feature is approved. > ++ Manu 0001-MINOR-ssl-skip-self-issued-CA-in-cert-chain-for-ssl_.patch Description: Binary data
Re: commit 493d9dc makes a SVN-checkout stall..
Hi guys, On Wed, Mar 25, 2020 at 01:42:14AM +0100, Olivier Houchard wrote: (...) > > Hope you have an idea where to look. Providing captures/logs is a bit > > difficult without some careful scrubbing.. I think a good test would be to just revert that patch, or selectively each one of the 3 lines it contains. > No answer yet, but I just tried to do a checkout of the FreeBSD svn tree > via haproxy, and it indeed doesn't work, it's a bit late right now, but > I'll have a look tomorrow :) OK thanks. I remember that you had some opinions on some of them at some point. However I'm tempted to think that if any of them needs to be changed, we're probably missing something somewhere which could indicate that other subscribes might randomly fail, because each time it's in h1_detach() and I don't see why we should always wake up if already subscribed. Thanks, Willy
