Fix #984 #1161
Merged
Fix #984 #1161
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
less bytes than the TLS overhead as well as allocate an extra skb.
1. @nsize was copy&pasted from tcp_fragment(), but the last one uses it only for fast path with skb w/o frags. 2. reserved_tailroom is in union with mark which we process separately, so the field isn't compatible with current Tempesta code. Also it's used for egress path only and we don't need it on ingress path where ss_skb_split() is called. 3. GSO segementation for skb wasn't accounted: make couple of comments in TLS code and initialize it for split skb. (Later kernel patch will bring small logic on it as well.) Some cleanups. -jN builds sometetimes still fail in libtdb/tdbq dependence (see commit 1fc007d).
make -j4 clean all.
1. accurately fix skb->truesize and TCP write memory in kernel by tcp_skb_unclone(); 2. __split_pgfrag_del() if we just move pointers, then we do not free TCP write memory, so do not change skb->truesize. 3. ss_skb_unroll(): truesize and data_len/len are completely different counters, so do not mix them in ss_skb_adjust_data_len(). By the way, during the tests I saw crazy skb overheads - truesize can be larger than len in tens kilobytes. The explanation for such overheads is various fragments stoling (e.g. our __split_pgfrag_del) and cloning. 4. cleanup: move ss_skb coalescing functions closer to their calls.
vankoven
approved these changes
Jan 28, 2019
| * set TSO segs to proper value on next iteration. | ||
| */ | ||
| t_sz = skb_tail->next->truesize; | ||
|
|
There was a problem hiding this comment.
I separated the lines to make the comment above relating to the line only.
| @@ -350,7 +350,7 @@ ss_do_send(struct sock *sk, struct sk_buff **skb_head, int flags) | |||
| int size, mss = tcp_send_mss(sk, &size, MSG_DONTWAIT); | |||
| unsigned int mark = (*skb_head)->mark; | |||
|
|
|||
| TFW_DBG3("[%d]: %s: sk=%p queue_empty=%d send_head=%p" | |||
| TFW_DBG3("[%d]: %s: sk=%pK queue_empty=%d send_head=%pK" | |||
There was a problem hiding this comment.
The year in the copyright notice isn't updated in the file. Can we just bump the year in all the sources just to forget about the problem?
There was a problem hiding this comment.
Just didn't update copyright for the very small changes. I think at the end it's not a problem at all if you just forget to update a copyright - if you or a reviewer recall this, then just update the copyright, otherwise if you forget - not a big deal.
aleksostapenko
approved these changes
Jan 28, 2019
less bytes than the TLS overhead as well as allocate an extra skb.
1. @nsize was copy&pasted from tcp_fragment(), but the last one uses it only for fast path with skb w/o frags. 2. reserved_tailroom is in union with mark which we process separately, so the field isn't compatible with current Tempesta code. Also it's used for egress path only and we don't need it on ingress path where ss_skb_split() is called. 3. GSO segementation for skb wasn't accounted: make couple of comments in TLS code and initialize it for split skb. (Later kernel patch will bring small logic on it as well.) Some cleanups. -jN builds sometetimes still fail in libtdb/tdbq dependence (see commit 1fc007d).
make -j4 clean all.
1. accurately fix skb->truesize and TCP write memory in kernel by tcp_skb_unclone(); 2. __split_pgfrag_del() if we just move pointers, then we do not free TCP write memory, so do not change skb->truesize. 3. ss_skb_unroll(): truesize and data_len/len are completely different counters, so do not mix them in ss_skb_adjust_data_len(). By the way, during the tests I saw crazy skb overheads - truesize can be larger than len in tens kilobytes. The explanation for such overheads is various fragments stoling (e.g. our __split_pgfrag_del) and cloning. 4. cleanup: move ss_skb coalescing functions closer to their calls.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Original PR is #1155 plus fixed code review comments and added fix of TSO segments. All the comments were addressed except #1155 (comment) - see motivation under the comment.