Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(280)

Side by Side Diff: net/quic/quic_stream_sequencer.cc

Issue 11300020: Add QuicStream and friends to QUIC code. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: License Created 8 years, 1 month ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch | Annotate | Revision Log
OLDNEW
(Empty)
1 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file.
4
5 #include "net/quic/quic_stream_sequencer.h"
6
7 #include <algorithm>
8 #include <limits>
9
10 #include "base/logging.h"
11 #include "net/quic/reliable_quic_stream.h"
12
13 using std::min;
14 using std::numeric_limits;
15
16 namespace net {
17
18 QuicStreamSequencer::QuicStreamSequencer(ReliableQuicStream* quic_stream)
19 : stream_(quic_stream),
20 num_bytes_consumed_(0),
21 max_frame_memory_(numeric_limits<size_t>::max()),
22 close_offset_(numeric_limits<QuicStreamOffset>::max()),
23 half_close_(true) {
24 }
25
26 QuicStreamSequencer::QuicStreamSequencer(size_t max_frame_memory,
27 ReliableQuicStream* quic_stream)
28 : stream_(quic_stream),
29 num_bytes_consumed_(0),
30 max_frame_memory_(max_frame_memory),
31 close_offset_(numeric_limits<QuicStreamOffset>::max()),
32 half_close_(true) {
33 if (max_frame_memory < kMaxPacketSize) {
34 LOG(DFATAL) << "Setting max frame memory to " << max_frame_memory
35 << ". Some frames will be impossible to handle.";
36 }
37 }
38
39 QuicStreamSequencer::~QuicStreamSequencer() {
40 }
41
42 bool QuicStreamSequencer::WillAcceptStreamFrame(
43 const QuicStreamFrame& frame) const {
44 QuicStreamOffset byte_offset = frame.offset;
45 size_t data_len = frame.data.size();
jar (doing other things) 2012/10/31 22:37:37 nit: declare/init as close to first use as possibl
Ryan Hamilton 2012/11/01 22:52:20 I assume you're referring to byte_offset, not data
jar (doing other things) 2012/11/01 23:20:38 I think my comment was mostly bogus. I saw this p
46 DCHECK_LE(data_len, max_frame_memory_);
47
48 if (byte_offset < num_bytes_consumed_ ||
49 frames_.find(byte_offset) != frames_.end()) {
50 return false;
51 }
52 if (data_len > max_frame_memory_) {
53 // We're never going to buffer this frame and we can't pass it up the
54 // stream might only consume part of it and we'd need a partial ack.
jar (doing other things) 2012/10/31 22:37:37 nit: typo? "...can't pass it up the stream might.
Ryan Hamilton 2012/11/01 22:52:20 Fixed upstream.
55 //
56 // Ideally this should never happen, as we check that
57 // max_frame_memory_ > kMaxPacketSize and lower levels should reject
58 // frames larger than that.
59 return false;
60 }
61 if (byte_offset + data_len - num_bytes_consumed_ > max_frame_memory_) {
62 // We can buffer this but not right now. Toss it.
63 // It might be worth trying an experiment where we try best-effort buffering
64 return false;
65 }
66 return true;
67 }
68
69 bool QuicStreamSequencer::OnStreamFrame(const QuicStreamFrame& frame) {
70 QuicStreamOffset byte_offset = frame.offset;
71 const char* data = frame.data.data();
72 size_t data_len = frame.data.size();
jar (doing other things) 2012/10/31 22:37:37 nit: define/init as close to first use as possible
Ryan Hamilton 2012/11/01 22:52:20 Fixed upstream.
73
74 if (!WillAcceptStreamFrame(frame)) {
75 // This should not happen, as WillAcceptFrame should be called before
jar (doing other things) 2012/10/31 22:37:37 Shouldn't this be a DCHECK(), even if we do put th
Ryan Hamilton 2012/11/01 22:52:20 Could be a DCHECK, but that complicates unit tests
76 // OnStreamFrame. Error handling should be done by the caller.
77 return false;
78 }
79
80 if (byte_offset == num_bytes_consumed_) {
81 DVLOG(1) << "Processing byte offset " << byte_offset;
82 size_t bytes_consumed = stream_->ProcessData(data, data_len);
83 num_bytes_consumed_ += bytes_consumed;
84
85 if (MaybeCloseStream()) {
86 return true;
87 }
88 if (bytes_consumed > data_len) {
89 stream_->Close(QUIC_SERVER_ERROR_PROCESSING_STREAM);
90 return false;
91 } else if (bytes_consumed == data_len) {
92 FlushBufferedFrames();
93 return true; // it's safe to ack this frame.
94 } else {
95 // Set ourselves up to buffer what's left
96 data_len -= bytes_consumed;
97 data += bytes_consumed;
98 byte_offset += bytes_consumed;
99 }
100 }
101
102 DVLOG(1) << "Buffering packet at offset " << byte_offset;
103 frames_.insert(make_pair(byte_offset, string(data, data_len)));
104 return true;
105 }
106
107 void QuicStreamSequencer::CloseStreamAtOffset(QuicStreamOffset offset,
108 bool half_close) {
109 const QuicStreamOffset kMaxOffset = numeric_limits<QuicStreamOffset>::max();
110
111 // If we have a scheduled termination or close, any new offset should match
112 // it.
113 if (close_offset_ != kMaxOffset && offset != close_offset_) {
114 stream_->Close(QUIC_MULTIPLE_TERMINATION_OFFSETS);
115 return;
116 }
117
118 close_offset_ = offset;
119 // Full close overrides half close.
120 if (half_close == false) {
121 half_close_ = false;
122 }
123
124 MaybeCloseStream();
125 }
126
127 bool QuicStreamSequencer::MaybeCloseStream() {
128 if (IsHalfClosed()) {
129 DVLOG(1) << "Passing up termination, as we've processed "
130 << num_bytes_consumed_ << " of " << close_offset_
131 << " bytes.";
132 // Technically it's an error if num_bytes_consumed isn't exactly
133 // equal, but error handling seems silly at this point.
134 stream_->TerminateFromPeer(half_close_);
135 return true;
136 }
137 return false;
138 }
139
140 size_t QuicStreamSequencer::GetReadableRegions(iovec* iov, size_t iov_len) {
141 FrameMap::iterator it = frames_.begin();
142 size_t idx = 0;
jar (doing other things) 2012/10/31 22:37:37 nit: idx --> index
Ryan Hamilton 2012/11/01 22:52:20 Fixed upstream.
143 QuicStreamOffset offset = num_bytes_consumed_;
144 while (it != frames_.end() && idx < iov_len) {
145 if (it->first != offset) return idx;
146
147 iov[idx].iov_base = static_cast<void*>(
148 const_cast<char*>(it->second.c_str()));
jar (doing other things) 2012/10/31 22:37:37 Should this be second.data()? I think the other c
alyssar 2012/11/01 17:43:47 +1 to data() just for clarity
Ryan Hamilton 2012/11/01 22:52:20 Fixed upstream.
149 iov[idx].iov_len = it->second.size();
150 offset += it->second.size();
151
152 ++idx;
153 ++it;
154 }
155 return idx;
156 }
157
158 void QuicStreamSequencer::AdvanceReadablePtr(size_t data_read) {
159 FrameMap::iterator it = frames_.begin();
jar (doing other things) 2012/10/31 22:37:37 nit: putting this after line 161 will mean that yo
Ryan Hamilton 2012/11/01 22:52:20 Fixed upstream.
160
161 while (data_read) {
162 if (it->first != num_bytes_consumed_ || it == frames_.end()) {
jar (doing other things) 2012/10/31 22:37:37 Shouldn't you first test for frames_.end()?
Ryan Hamilton 2012/11/01 22:52:20 Fixed upstream.
163 stream_->Close(QUIC_SERVER_ERROR_PROCESSING_STREAM); // Programming error
164 return;
165 }
166
167 if (data_read >= it->second.size()) {
168 data_read -= it->second.size();
169 num_bytes_consumed_ += it->second.size();
170 frames_.erase(it);
171 it = frames_.begin();
172 } else {
173 frames_.insert(make_pair(it->first + data_read,
174 it->second.substr(data_read)));
175 frames_.erase(frames_.begin());
176 num_bytes_consumed_ += data_read;
177 data_read = 0;
178 }
179 }
180 }
181
182 size_t QuicStreamSequencer::Readv(const struct iovec* iov, size_t iov_len) {
183 FrameMap::iterator it = frames_.begin();
184 size_t iov_idx = 0;
jar (doing other things) 2012/10/31 22:37:37 nit: iov_index? (I found other folks using the "
Ryan Hamilton 2012/11/01 22:52:20 Fixed upstream.
185 size_t iov_offset = 0;
186 size_t frame_offset = 0;
187 size_t initial_bytes_consumed = num_bytes_consumed_;
188
189 while (iov_idx < iov_len &&
190 it != frames_.end() &&
191 it->first == num_bytes_consumed_) {
192 size_t bytes_to_read = min(iov[iov_idx].iov_len - iov_offset,
193 it->second.size() - frame_offset);
194
195 char* iov_ptr = static_cast<char*>(iov[iov_idx].iov_base) + iov_offset;
jar (doing other things) 2012/10/31 22:37:37 This has me a bit scared. Some of the iov[].iov_b
alyssar 2012/11/01 17:43:47 I think there's some confusion about the memory mo
alyssar 2012/11/01 17:43:47 I think there's some confusion about the memory mo
Ryan Hamilton 2012/11/01 22:52:20 Changed to use data().
196 memcpy(iov_ptr,
197 it->second.c_str() + frame_offset, bytes_to_read);
198 frame_offset += bytes_to_read;
199 iov_offset += bytes_to_read;
200
201 if (iov[iov_idx].iov_len == iov_offset) {
202 // We've filled this buffer.
203 iov_offset = 0;
204 ++iov_idx;
205 }
206 if (it->second.size() == frame_offset) {
207 // We've copied this whole frame
208 num_bytes_consumed_ += it->second.size();
209 frames_.erase(it);
210 it = frames_.begin();
211 frame_offset = 0;
212 }
213 }
214 // We've finished copying. If we have a partial frame, update it.
215 if (frame_offset != 0) {
216 frames_.insert(make_pair(it->first + frame_offset,
217 it->second.substr(frame_offset)));
jar (doing other things) 2012/10/31 22:37:37 nit: indent
Ryan Hamilton 2012/11/01 22:52:20 Fixed upstream.
218 frames_.erase(frames_.begin());
219 num_bytes_consumed_ += frame_offset;
220 }
221 return num_bytes_consumed_ - initial_bytes_consumed;
jar (doing other things) 2012/10/31 22:37:37 The header says this should return the number of i
alyssar 2012/11/01 17:43:47 I believe the header should be updated. This is s
Ryan Hamilton 2012/11/01 22:52:20 Fixed upstream.
222 }
223
224 bool QuicStreamSequencer::HasBytesToRead() {
225 FrameMap::iterator it = frames_.begin();
226
227 return it != frames_.end() && it->first == num_bytes_consumed_;
228 }
229
230 bool QuicStreamSequencer::IsHalfClosed() {
231 return num_bytes_consumed_ >= close_offset_;
232 }
233
234 bool QuicStreamSequencer::IsClosed() {
235 return num_bytes_consumed_ >= close_offset_ && half_close_ == false;
236 }
237
238 void QuicStreamSequencer::FlushBufferedFrames() {
239 FrameMap::iterator it = frames_.find(num_bytes_consumed_);
240 while (it != frames_.end()) {
241 DVLOG(1) << "Flushing buffered packet at offset " << it->first;
242 string* data = &it->second;
243 size_t bytes_consumed = stream_->ProcessData(data->c_str(), data->size());
244 num_bytes_consumed_ += bytes_consumed;
245 if (MaybeCloseStream()) {
246 return;
247 }
248 if (bytes_consumed > data->size()) {
249 stream_->Close(QUIC_SERVER_ERROR_PROCESSING_STREAM); // Programming error
250 return;
251 } else if (bytes_consumed == data->size()) {
252 frames_.erase(it);
253 it = frames_.find(num_bytes_consumed_);
254 } else {
255 string new_data = it->second.substr(bytes_consumed);
256 frames_.erase(it);
257 frames_.insert(make_pair(num_bytes_consumed_, new_data));
jar (doing other things) 2012/10/31 22:37:37 I was curious why you used a slightly different pa
Ryan Hamilton 2012/11/01 22:52:20 Do you think something should be changed here? Ca
258 return;
259 }
260 }
261 }
262
263 } // namespace net
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698