Re: [PATCH] MEDIUM: sample: Implement sample fetch for arbitrary PROXY protocol v2 TLV values
On Tue, Aug 29, 2023 at 11:16:32PM +0200, ??? wrote: > ??, 29 ???. 2023 ?. ? 16:45, Willy Tarreau : > > > On Tue, Aug 29, 2023 at 04:31:31PM +0200, Willy Tarreau wrote: > > > On Tue, Aug 29, 2023 at 02:16:55PM +, Stephan, Alexander wrote: > > > > However, I noticed there is a problem now with the FreeBSD test. Have > > you > > > > already looked into it? > > > > > > Ah no, I had not noticed. I first pushed into a temporary branch and > > > everything was OK so I pushed into master again without checking since > > > it was the exact same commit. > > > > > > > I was not able to reproduce it for now. Looks like the test passes but > > it > > > > crashes afterwards? Maybe some SEGFAULT. Not sure... the CI on your > > branch > > > > looked good iirc. > > > > > > Hmmm no, signal 4 is SIGILL, we're using it for our asserts (BUG_ON()). > > > I never know how to retrieve the stderr log from these tests, there are > > > too many layers for me, I've been explained several times but cannot > > > memorize it. > > > > > > I'm rebuilding here on a freebsd machine, hoping to see it happen again. > > > > > > Note that we cannot rule out a transient issue such as a temporary memory > > > shortage or too long a CPU pause as such VMs are usually overloaded. > > > Maybe it has nothing to do with your new test but just randomly triggered > > > there, or maybe it put the light on a corner case. At least it's not a > > > segfault or such a crash that would indicate a pointer misuse. > > > > I'm getting totally random results from vtest on FreeBSD, I don't know > > why. I even tried to limit to one parallel process and am still getting > > on average 2 errors per run, most always on SSL but not only. Some of > > them include connection failures (sC) with 503 being returned and showing > > "Timeout during SSL handshake". I'm not getting the SIGILL though. Thus > > I don't know what to think about it, especially since it previously worked. > > We'll see if it happens again on next push. > > > > Willy > > > > we can disable cirrus-ci, not sure what is the purpose of randomly failing > CI :) I think it's still used for arm builds or something like this. Regardless, given that freebsd randomly fails on my local platform,there might be something else. I remember that cirrus used to fail some time ago, I don't know what its status is right now. Willy
Re: [PATCH] MEDIUM: sample: Implement sample fetch for arbitrary PROXY protocol v2 TLV values
вт, 29 авг. 2023 г. в 16:45, Willy Tarreau : > On Tue, Aug 29, 2023 at 04:31:31PM +0200, Willy Tarreau wrote: > > On Tue, Aug 29, 2023 at 02:16:55PM +, Stephan, Alexander wrote: > > > However, I noticed there is a problem now with the FreeBSD test. Have > you > > > already looked into it? > > > > Ah no, I had not noticed. I first pushed into a temporary branch and > > everything was OK so I pushed into master again without checking since > > it was the exact same commit. > > > > > I was not able to reproduce it for now. Looks like the test passes but > it > > > crashes afterwards? Maybe some SEGFAULT. Not sure... the CI on your > branch > > > looked good iirc. > > > > Hmmm no, signal 4 is SIGILL, we're using it for our asserts (BUG_ON()). > > I never know how to retrieve the stderr log from these tests, there are > > too many layers for me, I've been explained several times but cannot > > memorize it. > > > > I'm rebuilding here on a freebsd machine, hoping to see it happen again. > > > > Note that we cannot rule out a transient issue such as a temporary memory > > shortage or too long a CPU pause as such VMs are usually overloaded. > > Maybe it has nothing to do with your new test but just randomly triggered > > there, or maybe it put the light on a corner case. At least it's not a > > segfault or such a crash that would indicate a pointer misuse. > > I'm getting totally random results from vtest on FreeBSD, I don't know > why. I even tried to limit to one parallel process and am still getting > on average 2 errors per run, most always on SSL but not only. Some of > them include connection failures (sC) with 503 being returned and showing > "Timeout during SSL handshake". I'm not getting the SIGILL though. Thus > I don't know what to think about it, especially since it previously worked. > We'll see if it happens again on next push. > > Willy > we can disable cirrus-ci, not sure what is the purpose of randomly failing CI :)
Re: [PATCH] MEDIUM: sample: Implement sample fetch for arbitrary PROXY protocol v2 TLV values
On Tue, Aug 29, 2023 at 04:31:31PM +0200, Willy Tarreau wrote: > On Tue, Aug 29, 2023 at 02:16:55PM +, Stephan, Alexander wrote: > > However, I noticed there is a problem now with the FreeBSD test. Have you > > already looked into it? > > Ah no, I had not noticed. I first pushed into a temporary branch and > everything was OK so I pushed into master again without checking since > it was the exact same commit. > > > I was not able to reproduce it for now. Looks like the test passes but it > > crashes afterwards? Maybe some SEGFAULT. Not sure... the CI on your branch > > looked good iirc. > > Hmmm no, signal 4 is SIGILL, we're using it for our asserts (BUG_ON()). > I never know how to retrieve the stderr log from these tests, there are > too many layers for me, I've been explained several times but cannot > memorize it. > > I'm rebuilding here on a freebsd machine, hoping to see it happen again. > > Note that we cannot rule out a transient issue such as a temporary memory > shortage or too long a CPU pause as such VMs are usually overloaded. > Maybe it has nothing to do with your new test but just randomly triggered > there, or maybe it put the light on a corner case. At least it's not a > segfault or such a crash that would indicate a pointer misuse. I'm getting totally random results from vtest on FreeBSD, I don't know why. I even tried to limit to one parallel process and am still getting on average 2 errors per run, most always on SSL but not only. Some of them include connection failures (sC) with 503 being returned and showing "Timeout during SSL handshake". I'm not getting the SIGILL though. Thus I don't know what to think about it, especially since it previously worked. We'll see if it happens again on next push. Willy
Re: [PATCH] MEDIUM: sample: Implement sample fetch for arbitrary PROXY protocol v2 TLV values
On Tue, Aug 29, 2023 at 02:16:55PM +, Stephan, Alexander wrote: > However, I noticed there is a problem now with the FreeBSD test. Have you > already looked into it? Ah no, I had not noticed. I first pushed into a temporary branch and everything was OK so I pushed into master again without checking since it was the exact same commit. > I was not able to reproduce it for now. Looks like the test passes but it > crashes afterwards? Maybe some SEGFAULT. Not sure... the CI on your branch > looked good iirc. Hmmm no, signal 4 is SIGILL, we're using it for our asserts (BUG_ON()). I never know how to retrieve the stderr log from these tests, there are too many layers for me, I've been explained several times but cannot memorize it. I'm rebuilding here on a freebsd machine, hoping to see it happen again. Note that we cannot rule out a transient issue such as a temporary memory shortage or too long a CPU pause as such VMs are usually overloaded. Maybe it has nothing to do with your new test but just randomly triggered there, or maybe it put the light on a corner case. At least it's not a segfault or such a crash that would indicate a pointer misuse. Willy
RE: [PATCH] MEDIUM: sample: Implement sample fetch for arbitrary PROXY protocol v2 TLV values
Hi Willy, > And I was wrong, they were indeed for the first one. However I had to also > remove the NOT_LAST from the intermediate patches using the list_for_each(). > I put quotes around the symbolic names in the doc to make it clearer which > one was to be used and which one it corresponds to, as I had a moment of > hesitation when reading it the first time, so I profitted from this first > encounter to try to further limit doubts. Thanks for the adjustments and clarifications. > That's now merged, thanks again! Very happy that is merged now. However, I noticed there is a problem now with the FreeBSD test. Have you already looked into it? I was not able to reproduce it for now. Looks like the test passes but it crashes afterwards? Maybe some SEGFAULT. Not sure... the CI on your branch looked good iirc. Best, Alexander
Re: [PATCH] MEDIUM: sample: Implement sample fetch for arbitrary PROXY protocol v2 TLV values
On Tue, Aug 29, 2023 at 03:15:48PM +0200, Willy Tarreau wrote:
> Overall yes. There are just two small parts in the first patch that are
> for the immediately following patches ("refactor...") that I'm going to
> move there.
And I was wrong, they were indeed for the first one. However I had to
also remove the NOT_LAST from the intermediate patches using the
list_for_each(). I put quotes around the symbolic names in the doc to
make it clearer which one was to be used and which one it corresponds
to, as I had a moment of hesitation when reading it the first time, so
I profitted from this first encounter to try to further limit doubts.
> I'm doing that and merging it, or I'll get back to you if I have more
> questions. Thank you!
That's now merged, thanks again!
Willy
Re: [PATCH] MEDIUM: sample: Implement sample fetch for arbitrary PROXY protocol v2 TLV values
Hi Alexander,
On Mon, Aug 28, 2023 at 12:38:45PM +, Stephan, Alexander wrote:
> > I'm fine with this, however I find that the doc is not very clear about
> > what is permitted
> I agree that doc needs some more details. I added the note about the
> iterations and described all the symbolic constants, see at the end of this
> mail.
Thanks.
> > Unless you're having any objection, I'm going to flip type and len below:
> Sorry, that was an oversight on my end. Sure, go ahead. I actually thought I
> would have arranged it that way, but it somehow slipped my view.
No worries, that's what reviews are made for, isn't it ? ;-)
> > In this case it could be clarified in the doc that the sample fetch
> > functions for authority/uniqueid only return the first of each, and that
> > fc_pp_tlv() iterates over all occurrences of the requested ID. This would
> > then place a clear separation between trying to extract THE authority, or
> > checking ALL TLVs of type AUTHORITY.
>
> > What do you think ?
> Good idea, agreed. That also aligns with my goals to keep the existing
> behavior as much as possible.
OK great.
> > If you're OK with this, I'd appreciate it if you could send a few informal
> > incremental patches that I'd squash into yours.
> I'm OK with this. :-) The first patch (0001) should be squashed into the
> commit that introduces the fetch fc_pp_tlv.
> I guess it fits there because it actually retains and documents prior
> behavior. The behavior regarding duplicates is also already present,
> therefore I already added corresponding docs there.
> The second one (0002) could then be applied to the last patch with the
> introduction of the constants. Would that work for you?
Overall yes. There are just two small parts in the first patch that are
for the immediately following patches ("refactor...") that I'm going to
move there.
I'm doing that and merging it, or I'll get back to you if I have more
questions. Thank you!
Willy
