-
Notifications
You must be signed in to change notification settings - Fork 103
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
Websocket simple proxy protocol implementation #1595
Conversation
baa327a
to
eac58dd
Compare
f14cdf8
to
168129c
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.
Looks like quite a few places should be reworked
53dc7c6
to
c7e76e4
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.
Just reviewed #1594 comments in application to this PR.
This answers #1595 (comment) While for backend connection |
Signed-off-by: Aleksey Mikhaylov <aym@tempesta-tech.com>
Signed-off-by: Aleksey Mikhaylov <aym@tempesta-tech.com>
f460505
to
8dd6b95
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.
There are still several issues and several cleanups are also desired.
e7eda18
to
18a1c47
Compare
Signed-off-by: Aleksey Mikhaylov <aym@tempesta-tech.com>
a0b51d2
to
6fe99b0
Compare
Signed-off-by: Aleksey Mikhaylov <aym@tempesta-tech.com>
73779c5
to
6ef6324
Compare
Signed-off-by: Aleksey Mikhaylov <aym@tempesta-tech.com>
Signed-off-by: Aleksey Mikhaylov <aym@tempesta-tech.com>
0d27705
to
147ef9a
Compare
Co-authored-by: Alexander Krizhanovsky <ak@tempesta-tech.com>
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.
One more race in the new design, hopefully the last one. Please carefully verify my conclusions for any other hidden issues.
fb89d02
to
4057450
Compare
Signed-off-by: Aleksey Mikhaylov <aym@tempesta-tech.com>
4057450
to
ac84d06
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.
Good to merge. Just several minor cleanups/fixes are required.
@@ -200,7 +200,7 @@ typedef struct { | |||
unsigned char data_off; | |||
} TfwH2Ctx; | |||
|
|||
typedef struct TfwConn TfwConn; | |||
typedef struct tfw_conn_t TfwConn; | |||
|
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.
Seems some unnecessary change
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.
This line of code i had introduced in my earlier commits in previous PRs. It goes againts widly used practice in tempesta code. Mostly type named in snake case with _t
suffix. I have just fixed style.
? tfw_http_req_process(conn, stream, data->skb) | ||
: tfw_http_resp_process(conn, stream, data->skb); | ||
/* That is paired request, it may be freed after resp processing */ | ||
req = ((TfwHttpMsg *)stream->msg)->pair; |
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.
req
is also needed under the if
only
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.
It cannot be moved inside clause because it may be freed after resp processing.
Co-authored-by: Alexander Krizhanovsky <ak@tempesta-tech.com>
e50dc91
to
67cdf28
Compare
Signed-off-by: Aleksey Mikhaylov <aym@tempesta-tech.com>
67cdf28
to
6b4edba
Compare
Contributes to #755
Signed-off-by: Aleksey Mikhaylov aym@tempesta-tech.com