tcp: Redefined bind => listen and support for virtual IPs#1257
Merged
alfreb merged 5 commits intoincludeos:devfrom Mar 23, 2017
Merged
tcp: Redefined bind => listen and support for virtual IPs#1257alfreb merged 5 commits intoincludeos:devfrom
alfreb merged 5 commits intoincludeos:devfrom
Conversation
alfreb
requested changes
Mar 23, 2017
Contributor
alfreb
left a comment
alfreb
left a comment
There was a problem hiding this comment.
Nice! Will give us more options for virtual IP's, loopback etc. Only issue afaict is that it's now possible to bind to any address.
| // TODO: Avoid wrap around, increment ephemeral to next free port. | ||
| // while(is_bound(ephemeral_)) ++ephemeral_; // worst case is like 16k iterations :D | ||
| // need a solution that checks each word of the subset (the dynamic range) | ||
| Ensures(is_bound(ephemeral_) == false && "Hoped I wouldn't see the day..."); // this may happen... |
Contributor
Author
There was a problem hiding this comment.
Feel free to dig in xD
| // Stat increment packets dropped | ||
| packets_dropped_++; | ||
| debug("<TCP::drop> Packet dropped\n"); | ||
| // TODO: |
Contributor
There was a problem hiding this comment.
I'll add inet::is_valid_source(IP4::addr)
| // Stat increment packets dropped | ||
| packets_dropped_++; | ||
| debug("<TCP::drop> Packet dropped\n"); | ||
| // TODO: |
Contributor
There was a problem hiding this comment.
I'll add Inet::is_valid_source(IP4::addr)
| std::make_unique<tcp::Listener>(*this, socket, std::move(cb)) | ||
| ).first->second; | ||
| debug("<TCP::bind> Bound to port %i \n", port); | ||
| debug("<TCP::bind> Bound to socket %s \n", socket.to_string()); |
| } | ||
| auto& listener = listeners_.emplace(port, | ||
| std::make_unique<tcp::Listener>(*this, port, std::move(cb)) | ||
| bind(socket); |
Contributor
There was a problem hiding this comment.
What will happen on TCP::listen({{8, 8, 8, 8}, 80}? I think bind to socket has to verify the source IP with inet.
| * @brief Validate the address by making sure it's allowed in this context. | ||
| * | ||
| * @param[in] addr The address | ||
| */ |
Contributor
There was a problem hiding this comment.
Maybe refactor to validate_source(... addr)?
| { | ||
| if (UNLIKELY( is_bound(socket) )) | ||
| throw TCP_error{"Socket is already in use: " + socket.to_string()}; | ||
|
|
Contributor
There was a problem hiding this comment.
I think you need to call validate_address here.
alfreb
approved these changes
Mar 23, 2017
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
bind()is now an internal thing for TCP. What was earlier bind is nowlisten().It's now possible to set source address or source socket on both
listen()andconnect(). These may throw if they are unable to bind to the socket/address. Still missing the wiring with Inet, but added pseudo code:TCP::validate_address(ip4::Addr).