[
https://issues.apache.org/jira/browse/TS-4324?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15345674#comment-15345674
]
ASF GitHub Bot commented on TS-4324:
------------------------------------
Github user jpeach commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/733#discussion_r68171726
--- Diff: proxy/http2/Http2ClientSession.h ---
@@ -94,63 +89,60 @@ class Http2Frame
{
this->ioblock = new_IOBufferBlock();
this->ioblock->alloc(index);
- memcpy(this->ioblock->start(), this->hdr.raw, sizeof(this->hdr.raw));
- this->ioblock->fill(sizeof(this->hdr.raw));
-
- http2_parse_frame_header(make_iovec(this->ioblock->start(),
HTTP2_FRAME_HEADER_LEN), this->hdr.cooked);
}
- // Return the writeable buffer space.
+ // Return the writeable buffer space for frame payload
IOVec
write()
{
return make_iovec(this->ioblock->end(), this->ioblock->write_avail());
}
- // Once the frame has been serialized, update the length.
+ // Once the frame has been serialized, update the payload length of
frame header.
void
finalize(size_t nbytes)
{
if (this->ioblock) {
ink_assert((int64_t)nbytes <= this->ioblock->write_avail());
this->ioblock->fill(nbytes);
- this->hdr.cooked.length = this->ioblock->size() -
HTTP2_FRAME_HEADER_LEN;
- http2_write_frame_header(this->hdr.cooked,
make_iovec(this->ioblock->start(), HTTP2_FRAME_HEADER_LEN));
+ this->hdr.length = this->ioblock->size();
}
}
void
xmit(MIOBuffer *iobuffer)
{
- if (ioblock) {
+ // Write frame header
+ uint8_t buf[HTTP2_FRAME_HEADER_LEN];
+ http2_write_frame_header(hdr, make_iovec(buf));
+ iobuffer->write(buf, sizeof(buf));
+
+ // Write frame payload
+ // It could be empty (e.g. SETTINGS frame with ACK flag)
+ if (ioblock && ioblock->read_avail() > 0) {
iobuffer->append_block(this->ioblock.get());
- } else {
- iobuffer->write(this->hdr.raw, sizeof(this->hdr.raw));
}
}
int64_t
size()
{
if (ioblock) {
- return ioblock->size();
+ return HTTP2_FRAME_HEADER_LEN + ioblock->size();
} else {
- return sizeof(this->hdr.raw);
+ return HTTP2_FRAME_HEADER_LEN;
}
}
private:
Http2Frame(Http2Frame &); // noncopyable
Http2Frame &operator=(const Http2Frame &); // noncopyable
- Ptr<IOBufferBlock> ioblock;
IOBufferReader *ioreader;
- union {
- Http2FrameHeader cooked;
- uint8_t raw[HTTP2_FRAME_HEADER_LEN];
- } hdr;
+ Http2FrameHeader hdr; // frame header
+ Ptr<IOBufferBlock> ioblock; // frame payload
--- End diff --
Bikeshedding, but it would be nicer to keep ``ioreader`` down here near
``ioblock`` :)
> Inefficient way of transferring data frames
> -------------------------------------------
>
> Key: TS-4324
> URL: https://issues.apache.org/jira/browse/TS-4324
> Project: Traffic Server
> Issue Type: Improvement
> Components: HTTP/2
> Reporter: Bryan Call
> Assignee: Masaori Koshiba
> Fix For: 7.0.0
>
>
> ATS transfers data 8K - 9 bytes (http/2 header size) and then sends the 9
> bytes is couldn't write in a new frame that only has 9 bytes. This also
> happens if you bump up the buffer size to 16K.
> {code}
> [ 1.036] recv DATA frame <length=8183, flags=0x00, stream_id=13>
> [ 1.036] recv DATA frame <length=9, flags=0x00, stream_id=13>
> [ 1.259] recv DATA frame <length=8183, flags=0x00, stream_id=13>
> [ 1.259] recv DATA frame <length=9, flags=0x00, stream_id=13>
> [ 1.259] recv DATA frame <length=8183, flags=0x00, stream_id=13>
> [ 1.259] recv DATA frame <length=9, flags=0x00, stream_id=13>
> {code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
