Skip to content
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

Fix various problems reported in #1703 #1842

Merged
merged 3 commits into from
Apr 13, 2023
Merged

Fix various problems reported in #1703 #1842

merged 3 commits into from
Apr 13, 2023

Conversation

s0nx
Copy link
Contributor

@s0nx s0nx commented Mar 21, 2023

  • perform SKB linear data split during http/1 -> http/2 response transformation
  • with the attached linux patch, we shouldn't get low-headroom SKBs anymore

@s0nx s0nx self-assigned this Mar 21, 2023
@s0nx s0nx linked an issue Mar 21, 2023 that may be closed by this pull request
@s0nx s0nx marked this pull request as ready for review March 27, 2023 10:15
@s0nx s0nx changed the title fw/ss_skb: Perform SKB linear data split when doing Fix various problems reported in #1703. Mar 27, 2023
@s0nx s0nx changed the title Fix various problems reported in #1703. Fix various problems reported in #1703 Mar 27, 2023
@const-t const-t force-pushed the kt-1103-h2-msg-trasform branch 2 times, most recently from 7f2b22e to ea35810 Compare March 27, 2023 17:14
fw/http_msg.c Outdated Show resolved Hide resolved
fw/ss_skb.c Outdated Show resolved Hide resolved
fw/ss_skb.c Outdated Show resolved Hide resolved
fw/sock.c Outdated Show resolved Hide resolved
fw/ss_skb.h Outdated Show resolved Hide resolved
fw/ss_skb.h Outdated Show resolved Hide resolved
fw/http.h Outdated Show resolved Hide resolved
fw/http_msg.c Outdated Show resolved Hide resolved
Copy link
Contributor

@krizhanovsky krizhanovsky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The patch introduces a lot of sophisticated code, some of which seems duplicate of existing code, and additional computation complexity reducing performance of any traffic. All of this is just to fix a rare problem. Either an implementation of the idea must be much more polished or we should go with a simpler solution like catch the case wherever we can catch it, log it and close a TCP connection.

fw/sock.c Outdated Show resolved Hide resolved
fw/sock.c Outdated Show resolved Hide resolved
fw/ss_skb.c Outdated Show resolved Hide resolved
fw/sock.c Outdated Show resolved Hide resolved
fw/ss_skb.c Outdated Show resolved Hide resolved
fw/ss_skb.h Outdated Show resolved Hide resolved
fw/http.c Outdated Show resolved Hide resolved
fw/http_msg.c Show resolved Hide resolved
fw/ss_skb.c Outdated Show resolved Hide resolved
fw/ss_skb.c Show resolved Hide resolved
@const-t const-t force-pushed the kt-1103-h2-msg-trasform branch 3 times, most recently from ca26930 to 0913e9e Compare April 5, 2023 11:16
Base automatically changed from kt-1103-h2-msg-trasform to master April 6, 2023 10:50
@s0nx s0nx force-pushed the pv-1703-crash branch 2 times, most recently from 8d088ed to b87217b Compare April 10, 2023 11:00
fw/http_msg.c Outdated Show resolved Hide resolved
fw/ss_skb.c Show resolved Hide resolved
fw/ss_skb.c Outdated Show resolved Hide resolved
fw/ss_skb.h Outdated Show resolved Hide resolved
fw/ss_skb.c Outdated Show resolved Hide resolved
Copy link
Contributor

@krizhanovsky krizhanovsky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just don't have anything to add to @const-t and @EvgeniiMekhanik reviews, so I just approve the PR.

http/1 -> http/2 response transformation

Signed-off-by: Petr Vyazovik <[email protected]>
of outgoing SKBs during IPv4 fragmentation to overcome the issue
with the lack of space for transport headers

Signed-off-by: Petr Vyazovik <[email protected]>
@s0nx s0nx requested a review from const-t April 13, 2023 13:53
@krizhanovsky krizhanovsky merged commit 63f5607 into master Apr 13, 2023
@krizhanovsky krizhanovsky deleted the pv-1703-crash branch April 13, 2023 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Kernel crashes under concurrent connections tests
4 participants