-
Notifications
You must be signed in to change notification settings - Fork 6.6k
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
TCP memory leak fixes #23334
Merged
jukkar
merged 8 commits into
zephyrproject-rtos:master
from
jukkar:tcp-mem-leak-checking
Mar 17, 2020
Merged
TCP memory leak fixes #23334
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
bcc4122
net: pkt: Add flag for telling if this is 1st TCP packet
jukkar b6ce1d9
net: tcp: Fix memory leak when lot of incoming packets
jukkar deacf68
net: tcp: Print information when proper ACK is received
jukkar d4c0087
drivers: ethernet: stm32: Remove frag debug print in RX
jukkar 67832e0
net: pkt: Print caller and line when allocation fails
jukkar 0363de8
net: sockets: Release net_pkt if error during UDP recv()
jukkar 8790ad2
net: pkt: Print size of the failed net_buf allocation
jukkar 5b8a3c9
net: iface: Make sure we access valid ll address
jukkar File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
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
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.
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'd find both this code comment and commit message which introduces it ("net: pkt: Add flag for telling if this is TCP 1st packet") to be unclear and confusing. To start with, we already maintain linked list of packets which were already sent (and may need to be resent) as
tcp->sent_list
. So, if a packet is not in that list, it's "1st packet" (ahem to the terminology), and if it's in that list, it's "not 1st packet". So, do we need an extra flag for that? If we do, how about naming it more clear? E.g., "resent" (or "resent_pkt", or "pkt_resent"). But note that we already have flagssent_or_eof
andpkt_queued
. So, as I said, this is getting more and more confusing.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.
The flag tells is this the 1st time this packet has been sent. This is done in order to avoid doing one extra ref when sending it first time. It is indeed confusing, I try to clarify the documentation a bit. The
sent_or_eof
flag is for another purpose, there it means whether the pkt was sent in the first place. I tried to use the sent flag for detecting whether we need to ref the pkt in relevant places but it became very convoluted so it was much easier to just add a new flag for this. I agree that there are several flags now in the system and their interaction is hairy, but it is not easy to send the packet as there are two threads involved and the interaction is difficult when we have timers that expire when the packet is not even sent yet.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.
Ok, thanks for clarification.