-
Notifications
You must be signed in to change notification settings - Fork 105
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 #1189: correctly fill header part of skb when writing data into p… #1209
Conversation
58d16f0
to
cdaf090
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.
I made only high level review and a more accurate reviews about the frags and skb lists is required. I don't like the idea not to use skb head for big messages.
Please also backport the fixes to release-0.6
.
tempesta_fw/ss_skb.c
Outdated
if (!skb) | ||
return -ENOMEM; | ||
alloc_ldata = false; |
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 is no sense to generate more skbs if we can generate less. skb it self has large memory overhead and TCP segmentation also wins from larger skbs. It seems we can just resent it->frag
to -1 instead of 0 to fix #1189.
f_room = skb_tailroom(it->skb); | ||
n_copy = min(c_size, f_room); | ||
p = skb_put(it->skb, n_copy); | ||
} |
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 is a copy&paste from tfw_msg_write()
- can we reuse the code, e.g. by moving some of the code to a helper function?
…aged fragments ss_skb_frag_next() is always work with TfwMsgIter, so renamed and moved into msg.h, the whole TfwMsgIter became invalid if the function fails.
* add a new function, to allocate and add a new empty skb to current message iterator. * fix dereferences of invalid pointers in tfw_cache_build_resp_body(): it->frag may be equal to -1.
cdaf090
to
6b5e2c0
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 good to me.
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.
…aged fragments
See commit messages for the changes descriptions.