Skip to content

Move the postbuf into HttpSM #2766

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.

Sign up for GitHub

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

Merged
zwoop merged 1 commit into from
Nov 8, 2017
Merged

Move the postbuf into HttpSM #2766

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 67 additions & 21 deletions proxy/http/HttpSM.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1913,8 +1913,7 @@ HttpSM::state_read_server_response_header(int event, void *data)
if (enable_redirection && (redirection_tries <= t_state.txn_conf->number_of_redirections)) {
++redirection_tries;
} else {
tunnel.deallocate_redirect_postdata_buffers();
enable_redirection = false;
this->disable_redirect();
}

do_api_callout();
Expand Down Expand Up @@ -2693,7 +2692,7 @@ HttpSM::tunnel_handler_cache_fill(int event, void *data)
ink_release_assert(cache_sm.cache_write_vc);

tunnel.deallocate_buffers();
tunnel.deallocate_redirect_postdata_buffers();
this->postbuf_clear();
tunnel.reset();

setup_server_transfer_to_cache_only();
Expand All @@ -2720,7 +2719,7 @@ HttpSM::tunnel_handler_100_continue(int event, void *data)
// does not free the memory from the header
t_state.hdr_info.client_response.destroy();
tunnel.deallocate_buffers();
tunnel.deallocate_redirect_postdata_buffers();
this->postbuf_clear();
tunnel.reset();

if (server_entry->eos) {
Expand Down Expand Up @@ -2771,7 +2770,7 @@ HttpSM::tunnel_handler_push(int event, void *data)
// Reset tunneling state since we need to send a response
// to client as whether we succeeded
tunnel.deallocate_buffers();
tunnel.deallocate_redirect_postdata_buffers();
this->postbuf_clear();
tunnel.reset();

if (cache_write_success) {
Expand Down Expand Up @@ -3462,8 +3461,6 @@ HttpSM::tunnel_handler_for_partial_post(int event, void * /* data ATS_UNUSED */)
tunnel.deallocate_buffers();
tunnel.reset();

tunnel.allocate_redirect_postdata_producer_buffer();

Copy link
Member Author

@scw00 scw00 Nov 4, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line will cause the post buf truncated.

We should not deallocate postbuf. postdata_copy_buffer_start because it is the same as we doing postbuf.ua_buffer_reader->consume.

t_state.redirect_info.redirect_in_process = false;

if (post_failed) {
Expand Down Expand Up @@ -5258,8 +5255,7 @@ HttpSM::handle_post_failure()

// disable redirection in case we got a partial response and then EOS, because the buffer might not
// have the full post and it's deallocating the post buffers here
enable_redirection = false;
tunnel.deallocate_redirect_postdata_buffers();
this->disable_redirect();

// Don't even think about doing keep-alive after this debacle
t_state.client_info.keep_alive = HTTP_NO_KEEPALIVE;
Expand Down Expand Up @@ -5526,19 +5522,17 @@ HttpSM::do_setup_post_tunnel(HttpVC_t to_vc_type)
// YTS Team, yamsat Plugin
// if redirect_in_process and redirection is enabled add static producer

if (t_state.redirect_info.redirect_in_process && enable_redirection &&
(tunnel.postbuf && tunnel.postbuf->postdata_copy_buffer_start != nullptr &&
tunnel.postbuf->postdata_producer_buffer != nullptr)) {
if (t_state.redirect_info.redirect_in_process && enable_redirection && (this->_postbuf.postdata_copy_buffer_start != nullptr)) {
post_redirect = true;
// copy the post data into a new producer buffer for static producer
tunnel.postbuf->postdata_producer_buffer->write(tunnel.postbuf->postdata_copy_buffer_start);
int64_t post_bytes = tunnel.postbuf->postdata_producer_reader->read_avail();
MIOBuffer *postdata_producer_buffer = new_empty_MIOBuffer();
Copy link
Member

@oknet oknet Nov 4, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It saves the memory to avoid allocate a new IOBufferBlock because it always get data by clone().

IOBufferReader *postdata_producer_reader = postdata_producer_buffer->alloc_reader();

postdata_producer_buffer->write(this->_postbuf.postdata_copy_buffer_start);
int64_t post_bytes = postdata_producer_reader->read_avail();
transfered_bytes = post_bytes;
p = tunnel.add_producer(HTTP_TUNNEL_STATIC_PRODUCER, post_bytes, tunnel.postbuf->postdata_producer_reader,
(HttpProducerHandler) nullptr, HT_STATIC, "redirect static agent post");
// the tunnel has taken over the buffer and will free it
tunnel.postbuf->postdata_producer_buffer = nullptr;
tunnel.postbuf->postdata_producer_reader = nullptr;
p = tunnel.add_producer(HTTP_TUNNEL_STATIC_PRODUCER, post_bytes, postdata_producer_reader, (HttpProducerHandler) nullptr,
HT_STATIC, "redirect static agent post");
} else {
int64_t alloc_index;
// content length is undefined, use default buffer size
Expand All @@ -5554,6 +5548,10 @@ HttpSM::do_setup_post_tunnel(HttpVC_t to_vc_type)
IOBufferReader *buf_start = post_buffer->alloc_reader();
int64_t post_bytes = chunked ? INT64_MAX : t_state.hdr_info.request_content_length;

if (enable_redirection) {
this->_postbuf.init(post_buffer->clone_reader(buf_start));
}

// Note: Many browsers, Netscape and IE included send two extra
// bytes (CRLF) at the end of the post. We just ignore those
// bytes since the sending them is not spec
Expand Down Expand Up @@ -6695,7 +6693,7 @@ void
HttpSM::kill_this()
{
ink_release_assert(reentrancy_count == 1);
tunnel.deallocate_redirect_postdata_buffers();
this->postbuf_clear();
enable_redirection = false;

if (kill_this_async_done == false) {
Expand Down Expand Up @@ -7599,7 +7597,7 @@ HttpSM::do_redirect()
{
DebugSM("http_redirect", "[HttpSM::do_redirect]");
if (!enable_redirection || redirection_tries > t_state.txn_conf->number_of_redirections) {
tunnel.deallocate_redirect_postdata_buffers();
this->postbuf_clear();
return;
}

Expand Down Expand Up @@ -7988,3 +7986,51 @@ HttpSM::find_proto_string(HTTPVersion version) const
}
return nullptr;
}

// YTS Team, yamsat Plugin
// Function to copy the partial Post data while tunnelling
void
PostDataBuffers::copy_partial_post_data()
{
this->postdata_copy_buffer->write(this->ua_buffer_reader);
Debug("http_redirect", "[PostDataBuffers::copy_partial_post_data] wrote %" PRId64 " bytes to buffers %" PRId64 "",
this->ua_buffer_reader->read_avail(), this->postdata_copy_buffer_start->read_avail());
this->ua_buffer_reader->consume(this->ua_buffer_reader->read_avail());
}

// YTS Team, yamsat Plugin
// Allocating the post data buffers
void
PostDataBuffers::init(IOBufferReader *ua_reader)
{
Debug("http_redirect", "[PostDataBuffers::init]");

this->ua_buffer_reader = ua_reader;

if (this->postdata_copy_buffer == nullptr) {
ink_assert(this->postdata_copy_buffer_start == nullptr);
this->postdata_copy_buffer = new_empty_MIOBuffer(BUFFER_SIZE_INDEX_4K);
this->postdata_copy_buffer_start = this->postdata_copy_buffer->alloc_reader();
}

ink_assert(this->ua_buffer_reader != nullptr);
}

// YTS Team, yamsat Plugin
// Deallocating the post data buffers
void
PostDataBuffers::clear()
{
Debug("http_redirect", "[PostDataBuffers::clear]");

if (this->postdata_copy_buffer != nullptr) {
free_MIOBuffer(this->postdata_copy_buffer);
this->postdata_copy_buffer = nullptr;
this->postdata_copy_buffer_start = nullptr; // deallocated by the buffer
}
}

PostDataBuffers::~PostDataBuffers()
{
this->clear();
}
64 changes: 64 additions & 0 deletions proxy/http/HttpSM.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,22 @@ enum HttpPluginTunnel_t {
class CoreUtils;
class PluginVCCore;

class PostDataBuffers
{
public:
PostDataBuffers() { Debug("http_redirect", "[PostDataBuffers::PostDataBuffers]"); }

MIOBuffer *postdata_copy_buffer = nullptr;
IOBufferReader *postdata_copy_buffer_start = nullptr;
IOBufferReader *ua_buffer_reader = nullptr;

void clear();
void init(IOBufferReader *ua_reader);
void copy_partial_post_data();

~PostDataBuffers();
};
Copy link
Contributor

@d2r d2r Nov 4, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just an observation. By omitting the default constructor definition here, we are losing the following debug message from HttpTunnel.h

266      Debug("http_redirect", "[PostDataBuffers::PostDataBuffers]");

This may not have been a very useful debug message though.

Copy link
Member Author

@scw00 scw00 Nov 5, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO, postbuf in HttpSM is not a pointer. It is always allocated with HttpSM.

Copy link
Contributor

@d2r d2r Nov 6, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, sounds good. Just checking.


class HttpSM : public Continuation
{
friend class HttpPagesHandler;
Expand Down Expand Up @@ -296,6 +312,14 @@ class HttpSM : public Continuation

HttpTransact::State t_state;

// _postbuf api
int64_t postbuf_reader_avail();
int64_t postbuf_buffer_avail();
void postbuf_clear();
void disable_redirect();
void postbuf_copy_partial_data();
void postbuf_init(IOBufferReader *ua_reader);

protected:
int reentrancy_count = 0;

Expand Down Expand Up @@ -561,6 +585,9 @@ class HttpSM : public Continuation
{
return terminate_sm;
}

private:
PostDataBuffers _postbuf;
};

// Function to get the cache_sm object - YTS Team, yamsat
Expand Down Expand Up @@ -659,4 +686,41 @@ HttpSM::is_transparent_passthrough_allowed()
ua_session->get_transact_count() == 1);
}

inline int64_t
HttpSM::postbuf_reader_avail()
{
return this->_postbuf.ua_buffer_reader->read_avail();
}

inline int64_t
HttpSM::postbuf_buffer_avail()
{
return this->_postbuf.postdata_copy_buffer_start->read_avail();
}

inline void
HttpSM::postbuf_clear()
{
this->_postbuf.clear();
}

inline void
HttpSM::disable_redirect()
{
this->enable_redirection = false;
this->_postbuf.clear();
}

inline void
HttpSM::postbuf_copy_partial_data()
{
this->_postbuf.copy_partial_post_data();
}

inline void
HttpSM::postbuf_init(IOBufferReader *ua_reader)
{
this->_postbuf.init(ua_reader);
}

#endif
Loading