-
Notifications
You must be signed in to change notification settings - Fork 90
CONNECT/CONNECT-UDP Encapsulation Support #1379
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
46c5e51
to
fbe6068
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ca97948
to
eda8939
Compare
Signed-off-by: asingh-g <[email protected]>
Signed-off-by: asingh-g <[email protected]>
Signed-off-by: asingh-g <[email protected]>
Signed-off-by: asingh-g <[email protected]>
Signed-off-by: asingh-g <[email protected]>
Signed-off-by: asingh-g <[email protected]>
Signed-off-by: asingh-g <[email protected]>
Signed-off-by: asingh-g <[email protected]>
Signed-off-by: asingh-g <[email protected]>
Signed-off-by: asingh-g <[email protected]>
Signed-off-by: asingh-g <[email protected]>
Signed-off-by: asingh-g <[email protected]>
Signed-off-by: asingh-g <[email protected]>
Signed-off-by: asingh-g <[email protected]>
Signed-off-by: asingh-g <[email protected]>
Signed-off-by: asingh-g <[email protected]>
Signed-off-by: asingh-g <[email protected]>
Signed-off-by: asingh-g <[email protected]>
Signed-off-by: asingh-g <[email protected]>
Signed-off-by: asingh-g <[email protected]>
Signed-off-by: asingh-g <[email protected]>
Signed-off-by: asingh-g <[email protected]>
Signed-off-by: asingh-g <[email protected]>
Signed-off-by: asingh-g <[email protected]>
Signed-off-by: asingh-g <[email protected]>
Signed-off-by: asingh-g <[email protected]>
Signed-off-by: asingh-g <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still need to read over source/client/process_impl.cc
and the tests, but here's some comments.
@@ -1,5 +1,7 @@ | |||
#include "source/client/process_bootstrap.h" | |||
|
|||
#include <semaphore.h> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use the C++ semaphore
header rather than the C semaphore.h
header, or perhaps something from Abseil's synchronization library?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I looked into std::semaphore and abseil's synchronisation libs, I am not convinced if they're meant for IPC. The pshared flag (PTHREAD_PROCESS_SHARED) in semaphore.h is what allows us to do inter process synchronisation and it looks to me that both of the alternatives use PTHREAD_PROCESS_PRIVATE
Signed-off-by: asingh-g <[email protected]>
Signed-off-by: asingh-g <[email protected]>
Signed-off-by: asingh-g <[email protected]>
Signed-off-by: asingh-g <[email protected]>
Signed-off-by: asingh-g <[email protected]>
Signed-off-by: asingh-g <[email protected]>
Signed-off-by: asingh-g <[email protected]>
Signed-off-by: asingh-g <[email protected]>
b0b330f
to
47d662f
Compare
Signed-off-by: asingh-g <[email protected]>
Signed-off-by: asingh-g <[email protected]>
The tsan test timed out. Could you try something like |
Signed-off-by: asingh-g <[email protected]>
Hmm seems like its still timing out @eric846 |
We probably need to increase the timeout to fit the extra tests. But it seems nontrivial to increase the timeout.
|
Sorry for the confusion. I found these conflicting timeout settings:
Could you try editing line 6 of
and line 220 of
(commenting it out with our standard Hopefully the tsan test will succeed within 3600 seconds. Once it's working we can reduce the 3600 to something closer to the true duration. Why it was like this: I believe line 220 from Envoy's |
Signed-off-by: asingh-g <[email protected]>
We still got a 30m timeout despite adjusting I poked around a bit more. Check out
( Could one of the new integration testcases be getting stuck? And could we have an implicit 30m timeout somewhere in the test? The fact that it timed out during one of the new tests could indicate a problem with those tests. Up to now, my assumption has been that the total duration of all the integration tests just exceeded the allocated time. But what if the new integration tests uniquely get stuck in tsan mode? I have a couple of ideas for experiments:
|
To be able to load test Terminating HTTP CONNECT/CONNECT-UDP Proxies, I've implemented encapsulation by spinning up a second envoy as a subprocess and routing nighthawk's generated traffic through it.
Presently CONNECT-UDP is only supported for HTTP/2 in envoy, and CONNECT is supported for all of H1, H2 and H3 QUIC.
Therefore only following combinations are supported
H1,H2 over H1,H2,H3
and
H3 over H2
using a combination of the --protocol and --tunnel-protocol flags.